shithub: choc

Download patch

ref: fe9adcbd0b0e9b8e6ac744967bbf0ce3fcf40546
parent: 4ec7962877652b2ea56a9ae50ee8ff1921e06290
author: Simon Howard <fraggle@soulsphere.org>
date: Fri Jan 5 15:02:06 EST 2018 

hexen: Add validation of lump offset values.

Offsets like these (used for eg. CmdGoto instructions) should point to
a location inside the lump; otherwise it is an error and we should fail
an assertion.


--- a/src/hexen/p_acs.c
+++ b/src/hexen/p_acs.c
@@ -176,6 +176,7 @@
 
 int ACScriptCount;
 byte *ActionCodeBase;
+static int ActionCodeSize;
 acsInfo_t *ACSInfo;
 int MapVars[MAX_ACS_MAP_VARS];
 int WorldVars[MAX_ACS_WORLD_VARS];
@@ -326,6 +327,23 @@
 
 //==========================================================================
 //
+// ValidateLumpOffset
+//
+// Check that the given offset is a valid location inside the loaded ACS
+// lump and does not point outside it.
+//
+//==========================================================================
+
+static int ValidateLumpOffset(int offset, char *where)
+{
+    ACSAssert(offset >= 0, "negative lump offset in %s", where);
+    ACSAssert(offset < ActionCodeSize, "invalid lump offset in %s: %d >= %d",
+              where, offset, ActionCodeSize);
+    return offset;
+}
+
+//==========================================================================
+//
 // P_LoadACScripts
 //
 //==========================================================================
@@ -339,9 +357,10 @@
 
     header = W_CacheLumpNum(lump, PU_LEVEL);
     ActionCodeBase = (byte *) header;
+    ActionCodeSize = W_LumpLength(lump);
     buffer = (int *) ((byte *) header + LONG(header->infoOffset));
 
-    ACScriptCount = LONG(*buffer); 
+    ACScriptCount = LONG(*buffer);
     ++buffer;
 
     if (ACScriptCount == 0)
@@ -356,7 +375,9 @@
         info->number = LONG(*buffer);
         ++buffer;
 
-        info->address = (int *) ((byte *) ActionCodeBase + LONG(*buffer));
+        info->address = (int *) (
+            (byte *) ActionCodeBase +
+            ValidateLumpOffset(LONG(*buffer), "script header"));
         ++buffer;
 
         info->argCount = LONG(*buffer);
@@ -390,7 +411,8 @@
 
     for (i=0; i<ACStringCount; ++i)
     {
-        ACStrings[i] = (char *) ActionCodeBase + LONG(buffer[i]);
+        ACStrings[i] = (char *) ActionCodeBase +
+                       ValidateLumpOffset(LONG(buffer[i]), "string header");
     }
 
     memset(MapVars, 0, sizeof(MapVars));
@@ -1374,7 +1396,10 @@
 
 static int CmdGoto(void)
 {
-    PCodePtr = (int *) (ActionCodeBase + ReadCodeImmediate());
+    int offset;
+
+    offset = ValidateLumpOffset(ReadCodeImmediate(), "CmdGoto parameter");
+    PCodePtr = (int *) (ActionCodeBase + offset);
     return SCRIPT_CONTINUE;
 }
 
@@ -1382,7 +1407,7 @@
 {
     int offset;
 
-    offset = ReadCodeImmediate();
+    offset = ValidateLumpOffset(ReadCodeImmediate(), "CmdIfGoto parameter");
 
     if (Pop() != 0)
     {
@@ -1668,7 +1693,7 @@
 {
     int offset;
 
-    offset = ReadCodeImmediate();
+    offset = ValidateLumpOffset(ReadCodeImmediate(), "CmdIfNotGoto parameter");
 
     if (Pop() == 0)
     {
@@ -1712,7 +1737,7 @@
     int offset;
 
     value = ReadCodeImmediate();
-    offset = ReadCodeImmediate();
+    offset = ValidateLumpOffset(ReadCodeImmediate(), "CmdCaseGoto parameter");
 
     if (Top() == value)
     {