shithub: rgbds

Info  •  Files  •  Log  •  Branches

Add safeguards against string overflows

Use snprintf instead of other unsafe functions. That way it is possible
to limit the size of the buffer and to ensure that it never overflows.

Signed-off-by: Antonio Niño Díaz <[email protected]>

--- a/src/asm/asmy.y

+++ b/src/asm/asmy.y

@@ -1312,26 +1312,37 @@

 string		: T_STRING

 		{

-			strcpy($$, $1);

+			if (snprintf($$, MAXSTRLEN + 1, "%s", $1) > MAXSTRLEN)

+				warning("String is too long '%s'", $1);

 		}

 		| T_OP_STRSUB '(' string comma uconst comma uconst ')'

 		{

-			strncpy($$, $3 + $5 - 1, $7);

-			$$[$7] = 0;

+			uint32_t len = $7;

+			if (len > MAXSTRLEN) {

+				warning("STRSUB: Length too big: %u", len);

+				len = MAXSTRLEN;

+			}

+

+			if (snprintf($$, len + 1, "%s", $3 + $5 - 1) > MAXSTRLEN)

+				warning("STRSUB: String too long '%s'", $$);

 		}

 		| T_OP_STRCAT '(' string comma string ')'

 		{

-			strcpy($$, $3);

-			strcat($$, $5);

+			if (snprintf($$, MAXSTRLEN + 1, "%s%s", $3, $5) > MAXSTRLEN)

+				warning("STRCAT: String too long '%s%s'", $3, $5);

 		}

 		| T_OP_STRUPR '(' string ')'

 		{

-			strcpy($$, $3);

+			if (snprintf($$, MAXSTRLEN + 1, "%s", $3) > MAXSTRLEN)

+				warning("STRUPR: String too long '%s'", $3);

+

 			upperstring($$);

 		}

 		| T_OP_STRLWR '(' string ')'

 		{

-			strcpy($$, $3);

+			if (snprintf($$, MAXSTRLEN + 1, "%s", $3) > MAXSTRLEN)

+				warning("STRUPR: String too long '%s'", $3);

+

 			lowerstring($$);

 		}

 ;

--- a/src/asm/fstack.c

+++ b/src/asm/fstack.c

@@ -347,7 +347,7 @@

 	pushcontext();

 	nCurrentStatus = STAT_isMacroArg;

-	sprintf(tzCurrentFileName, "%c", (uint8_t)s);

+	snprintf(tzCurrentFileName, _MAX_PATH + 1, "%c", (uint8_t)s);

 	CurrentFlexHandle = yy_scan_bytes(sym, strlen(sym));

 	yy_switch_to_buffer(CurrentFlexHandle);

 }

@@ -410,7 +410,7 @@

 	nMacroCount = 0;

 	nCurrentStatus = STAT_isInclude;

-	strcpy(tzCurrentFileName, tzFileName);

+	snprintf(tzCurrentFileName, _MAX_PATH + 1, "%s", tzFileName);

 	CurrentFlexHandle = yy_create_buffer(pCurrentFile);

 	yy_switch_to_buffer(CurrentFlexHandle);

 	nLineNo = 1;

--- a/src/asm/symbol.c

+++ b/src/asm/symbol.c

@@ -40,7 +40,7 @@

 static char SavedTIMESTAMP_ISO8601_UTC[256];

 static char SavedDAY[3];

 static char SavedMONTH[3];

-static char SavedYEAR[5];

+static char SavedYEAR[20];

 static char SavedHOUR[3];

 static char SavedMINUTE[3];

 static char SavedSECOND[3];

@@ -122,7 +122,9 @@

 		return NULL;

 	}

-	strcpy((*ppsym)->tzName, s);

+	if (snprintf((*ppsym)->tzName, MAXSYMLEN + 1, "%s", s) > MAXSYMLEN)

+		warning("Symbol name is too long: '%s'", s);

+

 	(*ppsym)->nValue = 0;

 	(*ppsym)->nType = 0;

 	(*ppsym)->pScope = NULL;

@@ -130,7 +132,13 @@

 	(*ppsym)->pMacro = NULL;

 	(*ppsym)->pSection = NULL;

 	(*ppsym)->Callback = NULL;

-	strcpy((*ppsym)->tzFileName, tzCurrentFileName);

+

+	if (snprintf((*ppsym)->tzFileName, _MAX_PATH + 1, "%s",

+		     tzCurrentFileName) > _MAX_PATH) {

+		fatalerror("%s: File name is too long: '%s'", __func__,

+			   tzCurrentFileName);

+	}

+

 	(*ppsym)->nFileLine = fstk_GetLine();

 	return *ppsym;

 }

@@ -509,7 +517,7 @@

 {

 	char s[256];

-	sprintf(s, "_%u", nMacroCount);

+	snprintf(s, sizeof(s), "_%u", nMacroCount);

 	newmacroargs[MAXMACROARGS] = strdup(s);

 }

@@ -560,7 +568,7 @@

  *

  * If the desired symbol is a string it needs to be passed to this function with

  * quotes inside the string, like sym_AddString("name", "\"test\"), or the

- * assembler won't be able to use it with DB and similar. This is equivalent as

+ * assembler won't be able to use it with DB and similar. This is equivalent to

  * ``` name EQUS "\"test\"" ```

  *

  * If the desired symbol is a register or a number, just the terminator quotes

@@ -965,16 +973,22 @@

 		 * The '?' have to be escaped or they will be treated as

 		 * trigraphs...

 		 */

-		strcpy(SavedTIME, "\"\?\?:\?\?:\?\?\"");

-		strcpy(SavedDATE, "\"\?\? \?\?\? \?\?\?\?\"");

-		strcpy(SavedTIMESTAMP_ISO8601_LOCAL, "\"\?\?\?\?-\?\?-\?\?T\?\?:\?\?:\?\?+\?\?\?\?\"");

-		strcpy(SavedTIMESTAMP_ISO8601_UTC, "\"\?\?\?\?-\?\?-\?\?T\?\?:\?\?:\?\?Z\"");

-		strcpy(SavedDAY, "1");

-		strcpy(SavedMONTH, "1");

-		strcpy(SavedYEAR, "1900");

-		strcpy(SavedHOUR, "0");

-		strcpy(SavedMINUTE, "0");

-		strcpy(SavedSECOND, "0");

+		snprintf(SavedTIME, sizeof(SavedTIME),

+			 "\"\?\?:\?\?:\?\?\"");

+		snprintf(SavedDATE, sizeof(SavedDATE),

+			 "\"\?\? \?\?\? \?\?\?\?\"");

+		snprintf(SavedTIMESTAMP_ISO8601_LOCAL,

+			 sizeof(SavedTIMESTAMP_ISO8601_LOCAL),

+			 "\"\?\?\?\?-\?\?-\?\?T\?\?:\?\?:\?\?+\?\?\?\?\"");

+		snprintf(SavedTIMESTAMP_ISO8601_UTC,

+			 sizeof(SavedTIMESTAMP_ISO8601_UTC),

+			 "\"\?\?\?\?-\?\?-\?\?T\?\?:\?\?:\?\?Z\"");

+		snprintf(SavedDAY, sizeof(SavedDAY), "1");

+		snprintf(SavedMONTH, sizeof(SavedMONTH), "1");

+		snprintf(SavedYEAR, sizeof(SavedYEAR), "1900");

+		snprintf(SavedHOUR, sizeof(SavedHOUR), "0");

+		snprintf(SavedMINUTE, sizeof(SavedMINUTE), "0");

+		snprintf(SavedSECOND, sizeof(SavedSECOND), "0");

 	}

 	sym_AddString("__TIME__", SavedTIME);