missing type check for --/++ causes js.mem corruption

[profjohnhoff]

Hello,

I've stumbled upon a bug in do_op when handling TOK_POSTINC (and TOK_POSTDEC) assignment.
There's a missing type check for lhs, which causes it to be used directly instead of as an offset into variables (if lhs was an object).

Bug

static jsval_t do_assign_op(struct js *js, uint8_t op, jsval_t l, jsval_t r) {
...
jsval_t res = do_op(js, m[op - TOK_PLUS_ASSIGN], resolveprop(js, l), r);
assign(js, l, res);

in which resolveprop(js, l) will return l (T_NUM)

calling do_op with TOK_POSTINC or TOK_POSTDEC, is turned into do_assign_op(..., TOK_PLUS_ASSIGN|TOK_MINUS_ASSIGN, lhs, ...) with lhs being a number (so resolveprop(..., lhs) = lhs).

This causes OOB in struct js { uint8_t *mem; }.

Example

#include <stdio.h>
#include "elk.h"
int main(void) {
  char mem[200];
  struct js *js = js_create(mem, sizeof(mem));      // Create JS instance
  jsval_t result = js_eval(js, "1.1--;", ~0);       // Call sum
  printf("result: %s\n", js_str(js, result));       // result: 7
  return 0;
}

This causes a contrived memory corruption, bug somewhat controllable:

0x403d80 <saveval+32>    mov    qword ptr [rax + rdx], rdi

With rax pointing to js.mem, rdx set to 0x999999a0 and rdi to the full float 0x3fb99999999999a0

The corruption is given as:

*(js.mem+offset) = (val << 32) | offset;

Fix suggestion

Adding a type check to lhs before do_assign_op might be enough:

if (vtype(lhs) != T_OBJ && vtype(lhs) != T_CODEREF) return js_err(js, "bad lhs");

As such:

--- a/elk.c
+++ b/elk.c
@@ -154,6 +154,7 @@ static void saveoff(struct js *js, jsoff_t off, jsoff_t val) { memcpy(&js->mem[o
 static void saveval(struct js *js, jsoff_t off, jsval_t val) { memcpy(&js->mem[off], &val, sizeof(val)); }
 static jsoff_t loadoff(struct js *js, jsoff_t off) { jsoff_t v = 0; assert(js->brk <= js->size); memcpy(&v, &js->mem[off], sizeof(v)); return v; }
 static jsoff_t offtolen(jsoff_t off) { return (off >> 2) - 1; }
 static jsoff_t vstrlen(struct js *js, jsval_t v) { return offtolen(loadoff(js, vdata(v))); }
 static jsval_t loadval(struct js *js, jsoff_t off) { jsval_t v = 0; memcpy(&v, &js->mem[off], sizeof(v)); return v; }
 static jsval_t upper(struct js *js, jsval_t scope) { return mkval(T_OBJ, loadoff(js, vdata(scope) + sizeof(jsoff_t))); }
@@ -910,11 +911,23 @@ static jsval_t do_op(struct js *js, uint8_t op, jsval_t lhs, jsval_t rhs) {
     case TOK_TYPEOF:  return js_mkstr(js, typestr(vtype(r)), strlen(typestr(vtype(r))));
     case TOK_CALL:    return do_call_op(js, l, r);
     case TOK_ASSIGN:  return assign(js, lhs, r);
-    case TOK_POSTINC: { do_assign_op(js, TOK_PLUS_ASSIGN, lhs, tov(1)); return l; }
-    case TOK_POSTDEC: { do_assign_op(js, TOK_MINUS_ASSIGN, lhs, tov(1)); return l; }
+    case TOK_POSTINC: {
+
+                          if (vtype(lhs) != T_OBJ && vtype(lhs) != T_CODEREF) return js_err(js, "bad lhs");
+                          do_assign_op(js, TOK_PLUS_ASSIGN, lhs, tov(1));
+                          return l;
+                      }
+    case TOK_POSTDEC: {
+                          if (vtype(lhs) != T_OBJ && vtype(lhs) != T_CODEREF) return js_err(js, "bad lhs");
+                          do_assign_op(js, TOK_MINUS_ASSIGN, lhs, tov(1));
+                          return l;
+                      }
     case TOK_NOT:     if (vtype(r) == T_BOOL) return mkval(T_BOOL, !vdata(r)); break;
   }
-  if (is_assign(op))    return do_assign_op(js, op, lhs, r);
+  if (is_assign(op)) {
+    if (vtype(lhs) == T_OBJ || vtype(lhs) == T_CODEREF) return do_assign_op(js, op, lhs, r);
+    else return js_err(js, "bad lhs");
+  }
   if (vtype(l) == T_STR && vtype(r) == T_STR) return do_string_op(js, op, l, r);
   if (is_unary(op) && vtype(r) != T_NUM) return js_err(js, "type mismatch");
   if (!is_unary(op) && op != TOK_DOT && (vtype(l) != T_NUM || vtype(r) != T_NUM)) return js_err(js, "type mismatch");