shithub: rgbds

Download patch

ref: b019b0394621c130f53c0db03372ea036700270f
parent: 605acd24ba36e48948fd48e341c2c1523808abcc
parent: 74e9de1b71c0fd30899c48e0bdddcbb66482f17c
author: Antonio Niño Díaz <[email protected]>
date: Thu Jul 11 20:49:08 EDT 2019

Merge pull request #358 from dbrotz/fix-357

Fix buffer overflow when creating patches with long RPN expressions

--- a/include/asm/rpn.h
+++ b/include/asm/rpn.h
@@ -11,11 +11,14 @@
 
 #include <stdint.h>
 
+#define MAXRPNLEN 1048576
+
 struct Expression {
 	int32_t  nVal;
 	uint8_t  *tRPN;
 	uint32_t nRPNCapacity;
 	uint32_t nRPNLength;
+	uint32_t nRPNPatchSize;
 	uint32_t nRPNOut;
 	uint32_t isReloc;
 };
--- a/src/asm/output.c
+++ b/src/asm/output.c
@@ -10,6 +10,7 @@
  * Outputs an objectfile
  */
 
+#include <assert.h>
 #include <errno.h>
 #include <stdio.h>
 #include <stdint.h>
@@ -392,10 +393,15 @@
 {
 	struct Patch *pPatch;
 	uint16_t rpndata;
-	uint8_t rpnexpr[2048];
+	uint8_t *rpnexpr;
 	char tzSym[512];
 	uint32_t rpnptr = 0, symptr;
 
+	rpnexpr = malloc(expr->nRPNPatchSize);
+
+	if (rpnexpr == NULL)
+		fatalerror("No memory for patch RPN expression");
+
 	pPatch = allocpatch();
 	pPatch->nType = type;
 	strcpy(pPatch->tzFilename, tzCurrentFileName);
@@ -477,11 +483,10 @@
 		}
 	}
 
-	pPatch->pRPN = malloc(rpnptr);
-	if (pPatch->pRPN != NULL) {
-		memcpy(pPatch->pRPN, rpnexpr, rpnptr);
-		pPatch->nRPNSize = rpnptr;
-	}
+	assert(rpnptr == expr->nRPNPatchSize);
+
+	pPatch->pRPN = rpnexpr;
+	pPatch->nRPNSize = rpnptr;
 }
 
 /*
--- a/src/asm/rpn.c
+++ b/src/asm/rpn.c
@@ -27,7 +27,7 @@
 {
 	assert(src1->tRPN != NULL && src2->tRPN != NULL);
 
-	if (src1->nRPNLength > UINT32_MAX - src2->nRPNLength)
+	if (src1->nRPNLength + src2->nRPNLength > MAXRPNLEN)
 		fatalerror("RPN expression is too large");
 
 	uint32_t len = src1->nRPNLength + src2->nRPNLength;
@@ -43,7 +43,7 @@
 		uint32_t cap = (cap1 > cap2) ? cap1 : cap2;
 
 		if (len > cap)
-			cap = (cap <= UINT32_MAX / 2) ? cap * 2 : len;
+			cap = (cap <= MAXRPNLEN / 2) ? cap * 2 : MAXRPNLEN;
 
 		expr->nRPNCapacity = cap;
 		expr->tRPN = realloc(expr->tRPN, expr->nRPNCapacity);
@@ -55,6 +55,7 @@
 	free(src2->tRPN);
 
 	expr->nRPNLength = len;
+	expr->nRPNPatchSize = src1->nRPNPatchSize + src2->nRPNPatchSize;
 	expr->nRPNOut = 0;
 	expr->isReloc = src1->isReloc || src2->isReloc;
 }
@@ -69,8 +70,10 @@
 	if (expr->nRPNLength == expr->nRPNCapacity) {
 		if (expr->nRPNCapacity == 0)
 			expr->nRPNCapacity = 256;
-		else if (expr->nRPNCapacity > UINT32_MAX / 2)
+		else if (expr->nRPNCapacity == MAXRPNLEN)
 			fatalerror("RPN expression is too large");
+		else if (expr->nRPNCapacity > MAXRPNLEN / 2)
+			expr->nRPNCapacity = MAXRPNLEN;
 		else
 			expr->nRPNCapacity *= 2;
 		expr->tRPN = realloc(expr->tRPN, expr->nRPNCapacity);
@@ -90,6 +93,7 @@
 	expr->tRPN = NULL;
 	expr->nRPNCapacity = 0;
 	expr->nRPNLength = 0;
+	expr->nRPNPatchSize = 0;
 	expr->nRPNOut = 0;
 	expr->isReloc = 0;
 }
@@ -134,6 +138,7 @@
 	pushbyte(expr, i >> 16);
 	pushbyte(expr, i >> 24);
 	expr->nVal = i;
+	expr->nRPNPatchSize += 5;
 }
 
 void rpn_Symbol(struct Expression *expr, char *tzSym)
@@ -146,6 +151,7 @@
 		while (*tzSym)
 			pushbyte(expr, *tzSym++);
 		pushbyte(expr, 0);
+		expr->nRPNPatchSize += 5;
 	} else {
 		rpn_Number(expr, sym_GetConstantValue(tzSym));
 	}
@@ -162,6 +168,7 @@
 	expr->isReloc = 1;
 
 	pushbyte(expr, RPN_BANK_SELF);
+	expr->nRPNPatchSize++;
 }
 
 void rpn_BankSymbol(struct Expression *expr, char *tzSym)
@@ -180,6 +187,7 @@
 		while (*tzSym)
 			pushbyte(expr, *tzSym++);
 		pushbyte(expr, 0);
+		expr->nRPNPatchSize += 5;
 	} else {
 		yyerror("BANK argument must be a relocatable identifier");
 	}
@@ -196,9 +204,15 @@
 	expr->isReloc = 1;
 
 	pushbyte(expr, RPN_BANK_SECT);
-	while (*tzSectionName)
+	expr->nRPNPatchSize++;
+
+	while (*tzSectionName) {
 		pushbyte(expr, *tzSectionName++);
+		expr->nRPNPatchSize++;
+	}
+
 	pushbyte(expr, 0);
+	expr->nRPNPatchSize++;
 }
 
 void rpn_CheckHRAM(struct Expression *expr, const struct Expression *src)
@@ -205,6 +219,7 @@
 {
 	*expr = *src;
 	pushbyte(expr, RPN_HRAM);
+	expr->nRPNPatchSize++;
 }
 
 void rpn_LOGNOT(struct Expression *expr, const struct Expression *src)
@@ -211,6 +226,7 @@
 {
 	*expr = *src;
 	pushbyte(expr, RPN_LOGUNNOT);
+	expr->nRPNPatchSize++;
 }
 
 void rpn_LOGOR(struct Expression *expr, const struct Expression *src1,
@@ -219,6 +235,7 @@
 	joinexpr();
 	expr->nVal = (expr->nVal || src2->nVal);
 	pushbyte(expr, RPN_LOGOR);
+	expr->nRPNPatchSize++;
 }
 
 void rpn_LOGAND(struct Expression *expr, const struct Expression *src1,
@@ -227,6 +244,7 @@
 	joinexpr();
 	expr->nVal = (expr->nVal && src2->nVal);
 	pushbyte(expr, RPN_LOGAND);
+	expr->nRPNPatchSize++;
 }
 
 void rpn_HIGH(struct Expression *expr, const struct Expression *src)
@@ -250,6 +268,8 @@
 	pushbyte(expr, 0);
 
 	pushbyte(expr, RPN_AND);
+
+	expr->nRPNPatchSize += 12;
 }
 
 void rpn_LOW(struct Expression *expr, const struct Expression *src)
@@ -265,6 +285,8 @@
 	pushbyte(expr, 0);
 
 	pushbyte(expr, RPN_AND);
+
+	expr->nRPNPatchSize += 6;
 }
 
 void rpn_LOGEQU(struct Expression *expr, const struct Expression *src1,
@@ -273,6 +295,7 @@
 	joinexpr();
 	expr->nVal = (src1->nVal == src2->nVal);
 	pushbyte(expr, RPN_LOGEQ);
+	expr->nRPNPatchSize++;
 }
 
 void rpn_LOGGT(struct Expression *expr, const struct Expression *src1,
@@ -281,6 +304,7 @@
 	joinexpr();
 	expr->nVal = (src1->nVal > src2->nVal);
 	pushbyte(expr, RPN_LOGGT);
+	expr->nRPNPatchSize++;
 }
 
 void rpn_LOGLT(struct Expression *expr, const struct Expression *src1,
@@ -289,6 +313,7 @@
 	joinexpr();
 	expr->nVal = (src1->nVal < src2->nVal);
 	pushbyte(expr, RPN_LOGLT);
+	expr->nRPNPatchSize++;
 }
 
 void rpn_LOGGE(struct Expression *expr, const struct Expression *src1,
@@ -297,6 +322,7 @@
 	joinexpr();
 	expr->nVal = (src1->nVal >= src2->nVal);
 	pushbyte(expr, RPN_LOGGE);
+	expr->nRPNPatchSize++;
 }
 
 void rpn_LOGLE(struct Expression *expr, const struct Expression *src1,
@@ -305,6 +331,7 @@
 	joinexpr();
 	expr->nVal = (src1->nVal <= src2->nVal);
 	pushbyte(expr, RPN_LOGLE);
+	expr->nRPNPatchSize++;
 }
 
 void rpn_LOGNE(struct Expression *expr, const struct Expression *src1,
@@ -313,6 +340,7 @@
 	joinexpr();
 	expr->nVal = (src1->nVal != src2->nVal);
 	pushbyte(expr, RPN_LOGNE);
+	expr->nRPNPatchSize++;
 }
 
 void rpn_ADD(struct Expression *expr, const struct Expression *src1,
@@ -321,6 +349,7 @@
 	joinexpr();
 	expr->nVal = (src1->nVal + src2->nVal);
 	pushbyte(expr, RPN_ADD);
+	expr->nRPNPatchSize++;
 }
 
 void rpn_SUB(struct Expression *expr, const struct Expression *src1,
@@ -329,6 +358,7 @@
 	joinexpr();
 	expr->nVal = (src1->nVal - src2->nVal);
 	pushbyte(expr, RPN_SUB);
+	expr->nRPNPatchSize++;
 }
 
 void rpn_XOR(struct Expression *expr, const struct Expression *src1,
@@ -337,6 +367,7 @@
 	joinexpr();
 	expr->nVal = (src1->nVal ^ src2->nVal);
 	pushbyte(expr, RPN_XOR);
+	expr->nRPNPatchSize++;
 }
 
 void rpn_OR(struct Expression *expr, const struct Expression *src1,
@@ -345,6 +376,7 @@
 	joinexpr();
 	expr->nVal = (src1->nVal | src2->nVal);
 	pushbyte(expr, RPN_OR);
+	expr->nRPNPatchSize++;
 }
 
 void rpn_AND(struct Expression *expr, const struct Expression *src1,
@@ -353,6 +385,7 @@
 	joinexpr();
 	expr->nVal = (src1->nVal & src2->nVal);
 	pushbyte(expr, RPN_AND);
+	expr->nRPNPatchSize++;
 }
 
 void rpn_SHL(struct Expression *expr, const struct Expression *src1,
@@ -370,6 +403,7 @@
 
 	expr->nVal = (src1->nVal << src2->nVal);
 	pushbyte(expr, RPN_SHL);
+	expr->nRPNPatchSize++;
 }
 
 void rpn_SHR(struct Expression *expr, const struct Expression *src1,
@@ -383,6 +417,7 @@
 
 	expr->nVal = (src1->nVal >> src2->nVal);
 	pushbyte(expr, RPN_SHR);
+	expr->nRPNPatchSize++;
 }
 
 void rpn_MUL(struct Expression *expr, const struct Expression *src1,
@@ -391,6 +426,7 @@
 	joinexpr();
 	expr->nVal = (src1->nVal * src2->nVal);
 	pushbyte(expr, RPN_MUL);
+	expr->nRPNPatchSize++;
 }
 
 void rpn_DIV(struct Expression *expr, const struct Expression *src1,
@@ -402,6 +438,7 @@
 
 	expr->nVal = (src1->nVal / src2->nVal);
 	pushbyte(expr, RPN_DIV);
+	expr->nRPNPatchSize++;
 }
 
 void rpn_MOD(struct Expression *expr, const struct Expression *src1,
@@ -413,6 +450,7 @@
 
 	expr->nVal = (src1->nVal % src2->nVal);
 	pushbyte(expr, RPN_MOD);
+	expr->nRPNPatchSize++;
 }
 
 void rpn_UNNEG(struct Expression *expr, const struct Expression *src)
@@ -420,6 +458,7 @@
 	*expr = *src;
 	expr->nVal = -expr->nVal;
 	pushbyte(expr, RPN_UNSUB);
+	expr->nRPNPatchSize++;
 }
 
 void rpn_UNNOT(struct Expression *expr, const struct Expression *src)
@@ -427,4 +466,5 @@
 	*expr = *src;
 	expr->nVal = ~expr->nVal;
 	pushbyte(expr, RPN_UNNOT);
+	expr->nRPNPatchSize++;
 }
--- a/src/link/patch.c
+++ b/src/link/patch.c
@@ -18,13 +18,18 @@
 #include "link/mylink.h"
 #include "link/symbol.h"
 
+#define RPN_STACK_SIZE 256
+
 static struct sSection *pCurrentSection;
-static int32_t rpnstack[256];
+static int32_t rpnstack[RPN_STACK_SIZE];
 static int32_t rpnp;
 int32_t nPC;
 
 static void rpnpush(int32_t i)
 {
+	if (rpnp >= RPN_STACK_SIZE)
+		errx(1, "RPN stack overflow");
+
 	rpnstack[rpnp] = i;
 	rpnp++;
 }
--- /dev/null
+++ b/test/asm/long-rpn-expression.asm
@@ -1,0 +1,33 @@
+SECTION "sec", ROM0
+
+X0 EQUS "0"
+
+m: MACRO
+\1 EQUS STRCAT("{X\2}", "+0")
+ENDM
+
+n = 0
+
+REPT $7E
+n1 = n + 1
+NSTR EQUS STRSUB("{n}", 2, STRLEN("{n}") - 1)
+N1STR EQUS STRSUB("{n1}", 2, STRLEN("{n1}") - 1)
+XN1 EQUS STRCAT("X", "{N1STR}")
+    m XN1, {NSTR}
+    PURGE NSTR, N1STR, XN1
+n = n + 1
+ENDR
+
+; string of 127 zeros separated by plus signs
+X EQUS "{X7E}"
+
+    db x+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+\
+       X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+\
+       X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+\
+       X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+\
+       X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+\
+       X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+\
+       X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+\
+       X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X+X
+
+x db 0