Issue 296 - Avoid potential buffer overflow.

Author: wb2osz

src/encode_aprs.c

@@ -81,16 +81,23 @@ typedef struct position_s {
 
 static int set_norm_position (char symtab, char symbol, double dlat, double dlong, int ambiguity, position_t *presult)
 {
+	// An over zealous compiler might complain about l*itude_to_str writing
+	// N characters plus nul to an N character field so we stick it into a
+	// larger temp then copy the desired number of bytes.  (Issue 296)
 
-	latitude_to_str (dlat, ambiguity, presult->lat);
+	char stemp[16];
+
+	latitude_to_str (dlat, ambiguity, stemp);
+	memcpy (presult->lat, stemp, sizeof(presult->lat));
 
 	if (symtab != '/' && symtab != '\\' && ! isdigit(symtab) && ! isupper(symtab)) {
 	  text_color_set(DW_COLOR_ERROR);
 	  dw_printf ("Symbol table identifier is not one of / \\ 0-9 A-Z\n");
 	}
 	presult->sym_table_id = symtab;
 
-	longitude_to_str (dlong, ambiguity, presult->lon);
+	longitude_to_str (dlong, ambiguity, stemp);
+	memcpy (presult->lon, stemp, sizeof(presult->lon));
 
 	if (symbol < '!' || symbol > '~') {
 	  text_color_set(DW_COLOR_ERROR);

src/latlong.c

@@ -55,7 +55,11 @@
  * 		ambiguity	- If 1, 2, 3, or 4, blank out that many trailing digits.
  *
  * Outputs:	slat		- String in format ddmm.mm[NS]
- *				  Should always be exactly 8 characters + NUL.
+ *				  Must always be exactly 8 characters + NUL.
+ *				  Put in leading zeros if necessary.
+ *				  We must have exactly ddmm.mm and hemisphere because
+ *				  the APRS position report has fixed width fields.
+ *				  Trailing digits can be blanked for position ambiguity.
  *
  * Returns:     None
  *
@@ -101,13 +105,22 @@ void latitude_to_str (double dlat, int ambiguity, char *slat)
 	ideg = (int)dlat;
 	dmin = (dlat - ideg) * 60.;
 
+	// dmin is known to be in range of 0 <= dmin < 60.
+
+	// Minutes must be exactly like 99.99 with leading zeros,
+	// if needed, to make it fixed width.
+	// Two digits, decimal point, two digits, nul terminator.
+
 	snprintf (smin, sizeof(smin), "%05.2f", dmin);
 	/* Due to roundoff, 59.9999 could come out as "60.00" */
 	if (smin[0] == '6') {
 	  smin[0] = '0';
 	  ideg++;
 	}
 
+	// Assumes slat can hold 8 characters + nul.
+	// Degrees must be exactly 2 digits, with leading zero, if needed.
+
 	sprintf (slat, "%02d%s%c", ideg, smin, hemi);
 
 	if (ambiguity >= 1) {
@@ -135,9 +148,12 @@ void latitude_to_str (double dlat, int ambiguity, char *slat)
  * Inputs:      dlong		- Floating point degrees.
  * 		ambiguity	- If 1, 2, 3, or 4, blank out that many trailing digits.
  *
- * Outputs:	slat		- String in format dddmm.mm[NS]
- *				  Should always be exactly 9 characters + NUL.
- *
+ * Outputs:	slong		- String in format dddmm.mm[NS]
+ *				  Must always be exactly 9 characters + NUL.
+ *				  Put in leading zeros if necessary.
+ *				  We must have exactly dddmm.mm and hemisphere because
+ *				  the APRS position report has fixed width fields.
+ *				  Trailing digits can be blanked for position ambiguity.
  * Returns:     None
  *
  *----------------------------------------------------------------*/
@@ -178,7 +194,11 @@ void longitude_to_str (double dlong, int ambiguity, char *slong)
 	  ideg++;
 	}
 
+	// Assumes slong can hold 9 characters + nul.
+	// Degrees must be exactly 3 digits, with leading zero, if needed.
+
 	sprintf (slong, "%03d%s%c", ideg, smin, hemi);
+
 /*
  * The spec says position ambiguity in latitude also
  * applies to longitude automatically.  
@@ -903,6 +923,14 @@ int main (int argc, char *argv[])
 	latitude_to_str (45.999830, 4, result);
 	if (strcmp(result, "45  .  N") != 0) { errors++; dw_printf ("Error 1.8: Did not expect \"%s\"\n", result); }
 
+	// Test for leading zeros for small values.  Result must be fixed width.
+
+	latitude_to_str (0.016666666, 0, result);
+	if (strcmp(result, "0001.00N") != 0) { errors++; dw_printf ("Error 1.9: Did not expect \"%s\"\n", result); }
+
+	latitude_to_str (-1.999999, 0, result);
+	if (strcmp(result, "0200.00S") != 0) { errors++; dw_printf ("Error 1.10: Did not expect \"%s\"\n", result); }
+
 /* Longitude to APRS format. */
 
 	longitude_to_str (45.25, 0, result);
@@ -931,6 +959,15 @@ int main (int argc, char *argv[])
 	longitude_to_str (45.999830, 4, result);
 	if (strcmp(result, "045  .  E") != 0) { errors++; dw_printf ("Error 2.8: Did not expect \"%s\"\n", result); }
 
+	// Test for leading zeros for small values.  Result must be fixed width.
+
+	longitude_to_str (0.016666666, 0, result);
+	if (strcmp(result, "00001.00E") != 0) { errors++; dw_printf ("Error 2.9: Did not expect \"%s\"\n", result); }
+
+	longitude_to_str (-1.999999, 0, result);
+	if (strcmp(result, "00200.00W") != 0) { errors++; dw_printf ("Error 2.10: Did not expect \"%s\"\n", result); }
+
+
 /* Compressed format. */
 /* Protocol spec example has <*e7 but I got <*e8 due to rounding rather than truncation to integer. */