Merge pull request #17 from Jamiras/rp_fix

Prevent null reference if rich presence macro doesn't have a parameter
RetroAchievements · May 4, 2019 · 85c676d · 85c676d
2 parents bdaf7c6 + 174df53
commit 85c676d
Show file tree

Hide file tree

Showing 2 changed files with 88 additions and 2 deletions.
diff --git a/src/rcheevos/richpresence.c b/src/rcheevos/richpresence.c
@@ -103,6 +103,11 @@ static rc_richpresence_display_t* rc_parse_richpresence_display_internal(const c
       while (ptr < endline && *ptr != '(')
         ++ptr;
 
+      if (ptr == endline) {
+        parse->offset = RC_MISSING_VALUE;
+        return 0;
+      }
+
       if (ptr > line) {
         if (!parse->buffer) {
           /* just calculating size, can't confirm lookup exists */
@@ -179,6 +184,7 @@ static const char* rc_parse_richpresence_lookup(rc_richpresence_lookup_t* lookup
   const char* line;
   const char* endline;
   const char* defaultlabel = 0;
+  char* endptr = 0;
   unsigned key;
   int chars;
 
@@ -208,9 +214,14 @@ static const char* rc_parse_richpresence_lookup(rc_richpresence_lookup_t* lookup
       }
 
       if (number[0] == '0' && number[1] == 'x')
-        key = strtoul(&number[2], 0, 16);
+        key = strtoul(&number[2], &endptr, 16);
       else
-        key = strtoul(&number[0], 0, 10);
+        key = strtoul(&number[0], &endptr, 10);
+
+      if (*endptr && !isspace(*endptr)) {
+        parse->offset = RC_INVALID_CONST_OPERAND;
+        return nextline;
+      }
 
       item = RC_ALLOC(rc_richpresence_lookup_item_t, parse);
       item->value = key;
@@ -263,6 +274,8 @@ void rc_parse_richpresence_internal(rc_richpresence_t* self, const char* script,
       nextlookup = &lookup->next;
 
       nextline = rc_parse_richpresence_lookup(lookup, nextline, parse);
+      if (parse->offset < 0)
+        return;
 
     } else if (strncmp(line, "Format:", 7) == 0) {
       line += 7;
@@ -316,6 +329,8 @@ void rc_parse_richpresence_internal(rc_richpresence_t* self, const char* script,
 
       if (ptr < endline) {
         *nextdisplay = rc_parse_richpresence_display_internal(ptr + 1, endline, parse, self);
+        if (parse->offset < 0)
+          return;
         trigger = &((*nextdisplay)->trigger);
         rc_parse_trigger_internal(trigger, &line, parse);
         trigger->memrefs = 0;

diff --git a/test/test.c b/test/test.c
@@ -1373,6 +1373,32 @@ static void test_trigger(void) {
     assert(condset_get_cond(trigger_get_set(trigger, 0), 1)->current_hits == 3U);
   }
 
+  {
+    /*------------------------------------------------------------------------
+    TestAddHitsNoHitCount
+    Odd use case: AddHits a=1
+                          b=1
+    Since b=1 doesn't have a hitcount, it ignores the hits tallied by a=1
+    ------------------------------------------------------------------------*/
+
+    unsigned char ram[] = {0x00, 0x12, 0x34, 0xAB, 0x56};
+    memory_t memory;
+    rc_trigger_t* trigger;
+
+    memory.ram = ram;
+    memory.size = sizeof(ram);
+
+    parse_trigger(&trigger, buffer, "C:0xH0001=18_0xH0000=1");
+    comp_trigger(trigger, &memory, 0);
+    assert(condset_get_cond(trigger_get_set(trigger, 0), 0)->current_hits == 1U);
+    assert(condset_get_cond(trigger_get_set(trigger, 0), 1)->current_hits == 0U);
+
+    ram[0] = 1;
+    comp_trigger(trigger, &memory, 1);
+    assert(condset_get_cond(trigger_get_set(trigger, 0), 0)->current_hits == 2U);
+    assert(condset_get_cond(trigger_get_set(trigger, 0), 1)->current_hits == 1U);
+  }
+
   {
     /*------------------------------------------------------------------------
     TestHitCountPauseIfResetIf
@@ -3222,6 +3248,26 @@ static void test_richpresence(void) {
     assert(result == 37);
   }
 
+  {
+    /*------------------------------------------------------------------------
+    TestMacroWithNoParameter
+    ------------------------------------------------------------------------*/
+    int result;
+
+    result = rc_richpresence_size("Format:Points\nFormatType=VALUE\n\nDisplay:\n@Points Points");
+    assert(result == RC_MISSING_VALUE);
+  }
+
+  {
+    /*------------------------------------------------------------------------
+    TestConditionalDisplayMacroWithNoParameter
+    ------------------------------------------------------------------------*/
+    int result;
+
+    result = rc_richpresence_size("Format:Points\nFormatType=VALUE\n\nDisplay:\n?0x0h0001=1?@Points Points\nDefault");
+    assert(result == RC_MISSING_VALUE);
+  }
+
   {
     /*------------------------------------------------------------------------
     TestEscapedMacro
@@ -3520,6 +3566,31 @@ static void test_richpresence(void) {
     assert(result == 6);
   }
 
+  {
+    /*------------------------------------------------------------------------
+    TestLookupInvalid
+    ------------------------------------------------------------------------*/
+    int result;
+
+    result = rc_richpresence_size("Lookup:Location\nOx0=Zero\n1=One\n\nDisplay:\nAt @Location(0xH0000)");
+    assert(result == RC_INVALID_CONST_OPERAND);
+
+    result = rc_richpresence_size("Lookup:Location\n0xO=Zero\n1=One\n\nDisplay:\nAt @Location(0xH0000)");
+    assert(result == RC_INVALID_CONST_OPERAND);
+
+    result = rc_richpresence_size("Lookup:Location\nZero=Zero\n1=One\n\nDisplay:\nAt @Location(0xH0000)");
+    assert(result == RC_INVALID_CONST_OPERAND);
+
+    result = rc_richpresence_size("Lookup:Location\n0=Zero\n1=One\n\nDisplay:\nAt @Location");
+    assert(result == RC_MISSING_VALUE);
+
+    result = rc_richpresence_size("Lookup:Location\n0=Zero\n1=One\n\nDisplay:\nAt @Location()");
+    assert(result == RC_INVALID_MEMORY_OPERAND);
+
+    result = rc_richpresence_size("Lookup:Location\n0=Zero\n1=One\n\nDisplay:\nAt @Location(Zero)");
+    assert(result == RC_INVALID_MEMORY_OPERAND);
+  }
+
   {
     /*------------------------------------------------------------------------
     TestRandomTextBetweenSections