rtodms(): fix potential buffer overflow not dealt with with PR OSGeo#…

…3431 (master only)
rouault · Nov 7, 2022 · 72784f0 · 72784f0
1 parent 1116e0c
commit 72784f0
Show file tree

Hide file tree

Showing 2 changed files with 44 additions and 9 deletions.
diff --git a/src/rtodms.cpp b/src/rtodms.cpp
@@ -45,10 +45,19 @@ rtodms(char *s, size_t sizeof_s, double r, int pos, int neg) {
 	int deg, min, sign;
 	char *ss = s;
 	double sec;
+	size_t sizeof_ss = sizeof_s;
 
 	if (r < 0) {
 		r = -r;
-		if  (!pos) { *ss++ = '-'; sign = 0; }
+		if  (!pos) {
+		    if( sizeof_s == 1 ) {
+		        *s = 0;
+		        return s;
+		    }
+		    sizeof_ss --;
+		    *ss++ = '-';
+		    sign = 0;
+		}
 		else sign = neg;
 	} else
 		sign = pos;
@@ -60,28 +69,30 @@ rtodms(char *s, size_t sizeof_s, double r, int pos, int neg) {
 	deg = (int)r;
 
 	if (dolong)
-		(void)snprintf(ss,sizeof_s,format,deg,min,sec,sign);
+		(void)snprintf(ss,sizeof_ss,format,deg,min,sec,sign);
 	else if (sec != 0.0) {
 		char *p, *q;
 		/* double prime + pos/neg suffix (if included) + NUL */
 		size_t suffix_len = sign ? 3 : 2;
 
-		(void)snprintf(ss,sizeof_s,format,deg,min,sec,sign);
+		(void)snprintf(ss,sizeof_ss,format,deg,min,sec,sign);
                 /* Replace potential decimal comma by decimal point for non C locale */
                 for( p = ss; *p != '\0'; ++p ) {
                     if( *p == ',' ) {
                         *p = '.';
                         break;
                     }
                 }
+        if( suffix_len > strlen(ss) )
+            return s;
 		for (q = p = ss + strlen(ss) - suffix_len; *p == '0'; --p) ;
 		if (*p != '.')
 			++p;
 		if (++q != p)
 			(void)memmove(p, q, suffix_len);
 	} else if (min)
-		(void)snprintf(ss,sizeof_s,"%dd%d'%c",deg,min,sign);
+		(void)snprintf(ss,sizeof_ss,"%dd%d'%c",deg,min,sign);
 	else
-		(void)snprintf(ss,sizeof_s,"%dd%c",deg, sign);
+		(void)snprintf(ss,sizeof_ss,"%dd%c",deg, sign);
 	return s;
 }
diff --git a/test/unit/gie_self_tests.cpp b/test/unit/gie_self_tests.cpp
@@ -360,7 +360,7 @@ TEST(gie, info_functions) {
     const PJ_ELLPS *ellps_list;
     const PJ_PRIME_MERIDIANS *pm_list;
 
-    char buf[40];
+    std::vector<char> buf(40);
     PJ *P;
     char arg[50] = {"+proj=utm; +zone=32; +ellps=GRS80"};
     PJ_COORD a;
@@ -377,7 +377,8 @@ TEST(gie, info_functions) {
 
     if (info.version[0] != '\0') {
         char tmpstr[64];
-        snprintf(tmpstr, sizeof(tmpstr), "%d.%d.%d", info.major, info.minor, info.patch);
+        snprintf(tmpstr, sizeof(tmpstr), "%d.%d.%d", info.major, info.minor,
+                 info.patch);
         ASSERT_EQ(std::string(info.version), std::string(tmpstr));
     }
     ASSERT_NE(std::string(info.release), "");
@@ -440,12 +441,35 @@ TEST(gie, info_functions) {
     ASSERT_EQ(std::string(init_info.name), "epsg");
 
     /* test proj_rtodms() and proj_dmstor() */
-    ASSERT_EQ(std::string("180dN"), proj_rtodms2(buf, sizeof(buf), M_PI, 'N', 'S'));
+    ASSERT_EQ(std::string("180dN"),
+              proj_rtodms2(&buf[0], buf.size(), M_PI, 'N', 'S'));
 
     ASSERT_EQ(proj_dmstor(&buf[0], NULL), M_PI);
 
     ASSERT_EQ(std::string("114d35'29.612\"S"),
-              proj_rtodms2(buf, sizeof(buf), -2.0, 'N', 'S'));
+              proj_rtodms2(&buf[0], buf.size(), -2.0, 'N', 'S'));
+
+    // buffer of just one byte
+    ASSERT_EQ(std::string(""), proj_rtodms2(&buf[0], 1, -2.0, 'N', 'S'));
+
+    // last character truncated
+    ASSERT_EQ(std::string("114d35'29.612\""),
+              proj_rtodms2(&buf[0], 15, -2.0, 'N', 'S'));
+
+    // just enough bytes to store the string and the terminating nul character
+    ASSERT_EQ(std::string("114d35'29.612\"S"),
+              proj_rtodms2(&buf[0], 16, -2.0, 'N', 'S'));
+
+    // buffer of just one byte
+    ASSERT_EQ(std::string(""), proj_rtodms2(&buf[0], 1, -2.0, 0, 0));
+
+    // last character truncated
+    ASSERT_EQ(std::string("-114d35'29.612"),
+              proj_rtodms2(&buf[0], 15, -2.0, 0, 0));
+
+    // just enough bytes to store the string and the terminating nul character
+    ASSERT_EQ(std::string("-114d35'29.612\""),
+              proj_rtodms2(&buf[0], 16, -2.0, 0, 0));
 
     /* we can't expect perfect numerical accuracy so testing with a tolerance */
     ASSERT_NEAR(-2.0, proj_dmstor(&buf[0], NULL), 1e-7);