Segmentation Fault From NULL Address Pointer

[danielbarry]

In the following method it is possible to run into a segmentation fault (commit 1451d9b92452a0cfa0be64ec298cba42cd5ed818):

2279 static void printIPv4Addresses(uint16_t portInHostOrder) {
2280     struct ifaddrs* addrs = NULL;
2281     getifaddrs(&addrs);
2282     struct ifaddrs* p = addrs;
2283     while (NULL != p) {
2284         if (p->ifa_addr->sa_family == AF_INET) {
2285             char hostname[256];
2286             getnameinfo(p->ifa_addr, sizeof(struct sockaddr_in), hostname, sizeof(hostname), NULL, 0, NI_NUMERICHOST);
2287             ews_printf("Probably listening on http://%s:%u\n", hostname, portInHostOrder);
2288         }
2289         p = p->ifa_next;
2290     }
2291     if (NULL != addrs) {
2292         freeifaddrs(addrs);
2293     }
2294 }

After calling getifaddrs() it is possible that p->ifa_addr == NULL as per the documentation http://man7.org/linux/man-pages/man3/getifaddrs.3.html :

The ifa_addr field points to a structure containing the interface address. (The sa_family subfield should be consulted to determine the format of the address structure.) This field may contain a null pointer.

As the following code relies on the pointer not being NULL and this method having no impact on the actual functionality of the server itself, I suggest a patch as follows:

From fddf18dab7cb938f9d3fc6b0751d90866d2de4b3 Mon Sep 17 00:00:00 2001
From: Dan <danbarry16@googlemail.com>
Date: Sun, 24 Mar 2019 01:08:25 +1300
Subject: [PATCH] Check for NULL pointer after calling getifaddrs()

---
 EmbeddableWebServer.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/EmbeddableWebServer.h b/EmbeddableWebServer.h
index 129548b..febf093 100755
--- a/EmbeddableWebServer.h
+++ b/EmbeddableWebServer.h
@@ -2281,7 +2281,7 @@ static void printIPv4Addresses(uint16_t portInHostOrder) {
     getifaddrs(&addrs);
     struct ifaddrs* p = addrs;
     while (NULL != p) {
-        if (p->ifa_addr->sa_family == AF_INET) {
+        if (p->ifa_addr != NULL && p->ifa_addr->sa_family == AF_INET) {
             char hostname[256];
             getnameinfo(p->ifa_addr, sizeof(struct sockaddr_in), hostname, sizeof(hostname), NULL, 0, NI_NUMERICHOST);
             ews_printf("Probably listening on http://%s:%u\n", hostname, portInHostOrder);
-- 
2.17.1

P.S. Thanks for this awesome project, we are using it as part of a local debug server for a humanoid robotics project: https://www.humanoid.science/

[hellerf]

Whoa that's cool! I will add the NULL check. I guess the address can be NULL for an interface. The patch you sent doesn't apply cleanly to my branch - if you would like to get your name directly in the commit history you can send me a pull request and I can merge it. Otherwise I will put you as in AUTHORS.txt for release 1.1

[danielbarry]

I will add the NULL check. I guess the address can be NULL for an interface.

Yeah, it took me by surprise too. It built and ran fine in my VM but crashed when running on my local machine. Maybe there is some weirdness about my setup, but it doesn't seem to harm to guard against the NULL appearing.

The patch you sent doesn't apply cleanly to my branch - if you would like to get your name directly in the commit history you can send me a pull request and I can merge it. Otherwise I will put you as in AUTHORS.txt for release 1.1

Creating a fork and then creating a PR from that is more hassle than it's worth for a single one line commit, the way you've done it seems fine.

[hellerf]

This is now in the dev-1.1.0 branch. Good luck with your robot competitions! (And thanks for the patch!)

(Commit 626248ef)

[hellerf]

This was released in 1.1.0