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

…(master only)
OSGeo · Nov 7, 2022 · 1b1589a · 1b1589a
1 parent 1116e0c
commit 1b1589a
Show file tree

Hide file tree

Showing 2 changed files with 39 additions and 7 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,13 +69,13 @@ 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 == ',' ) {
@@ -80,8 +89,8 @@ rtodms(char *s, size_t sizeof_s, double r, int pos, int neg) {
 		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
@@ -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,13 +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, sizeof(buf), 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'));
 
+    // buffer of just one byte
+    ASSERT_EQ(std::string(""), proj_rtodms2(buf, 1, -2.0, 'N', 'S'));
+
+    // last character truncated
+    ASSERT_EQ(std::string("114d35'29.612\""),
+              proj_rtodms2(buf, 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, 16, -2.0, 'N', 'S'));
+
+    // buffer of just one byte
+    ASSERT_EQ(std::string(""), proj_rtodms2(buf, 1, -2.0, 0, 0));
+
+    // last character truncated
+    ASSERT_EQ(std::string("-114d35'29.612"), proj_rtodms2(buf, 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, 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);