Fix interpolation/STRFMT overflow issues (#838)

Widths and fractional widths greater than 255 would overflow a
uint8_t and wrap around to smaller values.

Total formatted lengths greater than the avilable buffer size
would overflow it and potentially corrupt memory.

Fixes #830
Closes #831

include/asm/format.h

@@ -28,9 +28,9 @@ struct FormatSpec {
 	bool prefix;
 	bool alignLeft;
 	bool padZero;
-	uint8_t width;
+	size_t width;
 	bool hasFrac;
-	uint8_t fracWidth;
+	size_t fracWidth;
 	int type;
 	bool valid;
 };

src/asm/format.c

@@ -6,6 +6,7 @@
  * SPDX-License-Identifier: MIT
  */
 
+#include <assert.h>
 #include <inttypes.h>
 #include <math.h>
 #include <stdbool.h>
@@ -148,20 +149,25 @@ void fmt_PrintString(char *buf, size_t bufLen, struct FormatSpec const *fmt, cha
 	size_t len = strlen(value);
 	size_t totalLen = fmt->width > len ? fmt->width : len;
 
-	if (totalLen + 1 > bufLen) /* bufLen includes terminator */
+	if (totalLen > bufLen - 1) { /* bufLen includes terminator */
 		error("Formatted string value too long\n");
+		totalLen = bufLen - 1;
+		if (len > totalLen)
+			len = totalLen;
+	}
+	assert(len < bufLen && totalLen < bufLen && len <= totalLen);
 
-	size_t padLen = fmt->width > len ? fmt->width - len : 0;
+	size_t padLen = totalLen - len;
 
 	if (fmt->alignLeft) {
-		strncpy(buf, value, len < bufLen ? len : bufLen);
+		memcpy(buf, value, len);
-		for (size_t i = 0; i < totalLen && len + i < bufLen; i++)
+		for (size_t i = len; i < totalLen; i++)
-			buf[len + i] = ' ';
-	} else {
-		for (size_t i = 0; i < padLen && i < bufLen; i++)
 			buf[i] = ' ';
-		if (bufLen > padLen)
+	} else {
-			strncpy(buf + padLen, value, bufLen - padLen - 1);
+		for (size_t i = 0; i < padLen; i++)
+			buf[i] = ' ';
+		if (totalLen > padLen)
+			memcpy(buf + padLen, value, len);
 	}
 
 	buf[totalLen] = '\0';
@@ -221,12 +227,18 @@ void fmt_PrintNumber(char *buf, size_t bufLen, struct FormatSpec const *fmt, uin
 		/* Special case for fixed-point */
 
 		/* Default fractional width (C's is 6 for "%f"; here 5 is enough) */
-		uint8_t fracWidth = fmt->hasFrac ? fmt->fracWidth : 5;
+		size_t fracWidth = fmt->hasFrac ? fmt->fracWidth : 5;
 
 		if (fracWidth) {
+			if (fracWidth > 255) {
+				error("Fractional width %zu too long, limiting to 255\n",
+				      fracWidth);
+				fracWidth = 255;
+			}
+
 			char spec[16]; /* Max "%" + 5-char PRIu32 + ".%0255.f" + terminator */
 
-			snprintf(spec, sizeof(spec), "%%" PRIu32 ".%%0%d.f", fracWidth);
+			snprintf(spec, sizeof(spec), "%%" PRIu32 ".%%0%zu.f", fracWidth);
 			snprintf(valueBuf, sizeof(valueBuf), spec, value >> 16,
 				 (value % 65536) / 65536.0 * pow(10, fracWidth) + 0.5);
 		} else {
@@ -253,46 +265,47 @@ void fmt_PrintNumber(char *buf, size_t bufLen, struct FormatSpec const *fmt, uin
 
 	size_t totalLen = fmt->width > numLen ? fmt->width : numLen;
 
-	if (totalLen + 1 > bufLen) /* bufLen includes terminator */
+	if (totalLen > bufLen - 1) { /* bufLen includes terminator */
 		error("Formatted numeric value too long\n");
+		totalLen = bufLen - 1;
+		if (numLen > totalLen) {
+			len -= numLen - totalLen;
+			numLen = totalLen;
+		}
+	}
+	assert(numLen < bufLen && totalLen < bufLen && numLen <= totalLen && len <= numLen);
 
-	size_t padLen = fmt->width > numLen ? fmt->width - numLen : 0;
+	size_t padLen = totalLen - numLen;
+	size_t pos = 0;
 
 	if (fmt->alignLeft) {
-		size_t pos = 0;
+		if (sign)
-
-		if (sign && pos < bufLen)
 			buf[pos++] = sign;
-		if (prefix && pos < bufLen)
+		if (prefix)
 			buf[pos++] = prefix;
-
+		memcpy(buf + pos, valueBuf, len);
-		strcpy(buf + pos, valueBuf);
+		for (size_t i = pos + len; i < totalLen; i++)
-		pos += len;
+			buf[i] = ' ';
-
-		for (size_t i = 0; i < totalLen && pos + i < bufLen; i++)
-			buf[pos + i] = ' ';
 	} else {
-		size_t pos = 0;
-
 		if (fmt->padZero) {
 			/* sign, then prefix, then zero padding */
-			if (sign && pos < bufLen)
+			if (sign)
 				buf[pos++] = sign;
-			if (prefix && pos < bufLen)
+			if (prefix)
 				buf[pos++] = prefix;
-			for (size_t i = 0; i < padLen && pos < bufLen; i++)
+			for (size_t i = 0; i < padLen; i++)
 				buf[pos++] = '0';
 		} else {
 			/* space padding, then sign, then prefix */
-			for (size_t i = 0; i < padLen && pos < bufLen; i++)
+			for (size_t i = 0; i < padLen; i++)
 				buf[pos++] = ' ';
-			if (sign && pos < bufLen)
+			if (sign)
 				buf[pos++] = sign;
-			if (prefix && pos < bufLen)
+			if (prefix)
 				buf[pos++] = prefix;
 		}
-		if (bufLen > pos)
+		if (totalLen > pos)
-			strcpy(buf + pos, valueBuf);
+			memcpy(buf + pos, valueBuf, len);
 	}
 
 	buf[totalLen] = '\0';

src/asm/rgbasm.5

@@ -337,7 +337,7 @@ followed by one or more
 \[en]
 .Ql 9 .
 If specified, prints this many digits of a fixed-point fraction.
-Defaults to 5 digits.
+Defaults to 5 digits, maximum 255 digits.
 .It Ql <type> Ta Specifies the type of value.
 .El
 .Pp

test/asm/format-truncation.asm

@@ -0,0 +1,15 @@
+num equ 42
+fix equ 123.0
+str equs "hello"
+
+println "{#0260x:num}"
+println "{#-260x:num}"
+println "{0280.260f:fix}"
+println "{260s:str}"
+println "{-260s:str}"
+
+println "<{#0260x:num}>"
+println "<{#-260x:num}>"
+println "<{0280.260f:fix}>"
+println "<{260s:str}>"
+println "<{-260s:str}>"

test/asm/format-truncation.err

@@ -0,0 +1,35 @@
+ERROR: format-truncation.asm(5):
+    Formatted numeric value too long
+ERROR: format-truncation.asm(6):
+    Formatted numeric value too long
+ERROR: format-truncation.asm(7):
+    Fractional width 260 too long, limiting to 255
+ERROR: format-truncation.asm(7):
+    Formatted numeric value too long
+ERROR: format-truncation.asm(8):
+    Formatted string value too long
+ERROR: format-truncation.asm(9):
+    Formatted string value too long
+ERROR: format-truncation.asm(11):
+    Formatted numeric value too long
+warning: format-truncation.asm(11): [-Wlong-string]
+    String constant too long
+ERROR: format-truncation.asm(12):
+    Formatted numeric value too long
+warning: format-truncation.asm(12): [-Wlong-string]
+    String constant too long
+ERROR: format-truncation.asm(13):
+    Fractional width 260 too long, limiting to 255
+ERROR: format-truncation.asm(13):
+    Formatted numeric value too long
+warning: format-truncation.asm(13): [-Wlong-string]
+    String constant too long
+ERROR: format-truncation.asm(14):
+    Formatted string value too long
+warning: format-truncation.asm(14): [-Wlong-string]
+    String constant too long
+ERROR: format-truncation.asm(15):
+    Formatted string value too long
+warning: format-truncation.asm(15): [-Wlong-string]
+    String constant too long
+error: Assembly aborted (12 errors)!

test/asm/format-truncation.out

@@ -0,0 +1,10 @@
+$0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000002a
+$2a                                                                                                                                                                                                                                                            
+123.00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000
+                                                                                                                                                                                                                                                          hello
+hello                                                                                                                                                                                                                                                          
+<$0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000002
+<$2a                                                                                                                                                                                                                                                           
+<123.0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000
+<                                                                                                                                                                                                                                                          hell
+<hello                                                                                                                                                                                                                                                         

test/asm/interpolation-overflow.asm

@@ -0,0 +1,4 @@
+; It seems that \1 was the easiest way to notice the memory corruption that
+; resulted from this overflow
+x = 0
+{.99999999f:x}\1

test/asm/interpolation-overflow.err

@@ -0,0 +1,9 @@
+ERROR: interpolation-overflow.asm(4):
+    Fractional width 99999999 too long, limiting to 255
+ERROR: interpolation-overflow.asm(4):
+    Formatted numeric value too long
+warning: interpolation-overflow.asm(4): [-Wlarge-constant]
+    Precision of fixed-point constant is too large
+while expanding symbol "0.0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000"
+FATAL: interpolation-overflow.asm(4):
+    Macro argument '\1' not defined