Bug#757884: Functions nc_uriparse and ocuriparse causes segmentation faults on malformed URLs

[Martin Steghöfer]

Package: netcdf
Version: 4.1.3-7

The functions "nc_uriparse" in nc_uri.c and "ocuriparse" in ocuri.c 
(which are very, very similar to each other, almost copies) both don't 
check properly for malformed URIs. For example they cause a segmentation 
fault when given the URI "file:o/", "file://", "file://a" or 
"http://hostname/"; (the latter one indirectly through a null-pointer 
returned by the mentioned functions, which is then referenced in 
ocuribuild, when doing an "ocopen").

This causes bugs in downstream libraries and executables, e.g. in mincdump:
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=716119

As it's not fixed in the most recent upstream version (4.3.2), I tried 
to report the bug upstream (to the netcdf developers), but it looks like 
their bug tracking system is read-only, even after signing up (then what 
do they have it for anyway?). I'm gonna try via email now.

But as the Debian package is way behind upstream (sticking with 4.1.*), 
an upstream fix probably wouldn't help you, either. So I provided a 
patch to fix the problem.

Description: Fix checks for malformed URIs to avoid crashes
 The functions nc_uriparse and ocuriparse caused a segmentation fault
 when called on malformed URIs like "file:o/", "file://" or "file:///".
 This was due to: (1) An error in the check for the // part of the URI
 (between the "[protocol]:" and the hostname part), (2) a missing bail-out
 when the part containing file and constraints is missing. Besides, the fact
 that both functions set a null-pointer as file string when the file part
 was present, but empty (containing only one slash), caused a segmentation
 fault in ocuribuild when passing a URL like "http://domain.org/"; to ocopen.
Author: Martin Steghöfer <mar...@steghoefer.eu>

--- a/libdispatch/nc_uri.c
+++ b/libdispatch/nc_uri.c
@@ -115,7 +115,7 @@
     p += strlen(protocol);
 
     /* 5. skip // */
-    if(*p != '/' && *(p+1) != '/')
+    if(*p != '/' || *(p+1) != '/')
 	goto fail;
     p += 2;
 
@@ -123,6 +123,8 @@
     file = strchr(p,'/');
     if(file) {
 	*file++ = '\0'; /* warning: we just overwrote the leading / */
+    } else {
+        goto fail;
     }
 
     /* 7. extract any user:pwd */
@@ -165,12 +167,10 @@
         nc_uri->host = strdup(host);
     if(port && strlen(port) > 0)
         nc_uri->port = strdup(port);
-    if(file && strlen(file) > 0) {
-	/* Add back the leading / */
-        nc_uri->file = malloc(strlen(file)+2);
-	strcpy(nc_uri->file,"/");
-        strcat(nc_uri->file,file);
-    }
+    /* Add back the leading / */
+    nc_uri->file = malloc(strlen(file)+2);
+    strcpy(nc_uri->file,"/");
+    strcat(nc_uri->file,file);
     if(constraint && strlen(constraint) > 0)
         nc_uri->constraint = strdup(constraint);
     nc_urisetconstraints(nc_uri,constraint);
--- a/oc/ocuri.c
+++ b/oc/ocuri.c
@@ -115,7 +115,7 @@
     p += strlen(protocol);
 
     /* 5. skip // */
-    if(*p != '/' && *(p+1) != '/')
+    if(*p != '/' || *(p+1) != '/')
 	goto fail;
     p += 2;
 
@@ -123,6 +123,8 @@
     file = strchr(p,'/');
     if(file) {
 	*file++ = '\0'; /* warning: we just overwrote the leading / */
+    } else {
+        goto fail;
     }
 
     /* 7. extract any user:pwd */
@@ -165,12 +167,10 @@
         ocuri->host = strdup(host);
     if(port && strlen(port) > 0)
         ocuri->port = strdup(port);
-    if(file && strlen(file) > 0) {
-	/* Add back the leading / */
-        ocuri->file = malloc(strlen(file)+2);
-	strcpy(ocuri->file,"/");
-        strcat(ocuri->file,file);
-    }
+    /* Add back the leading / */
+    ocuri->file = malloc(strlen(file)+2);
+    strcpy(ocuri->file,"/");
+    strcat(ocuri->file,file);
     if(constraint && strlen(constraint) > 0)
         ocuri->constraint = strdup(constraint);
     ocurisetconstraints(ocuri,constraint);