CVE-2022-25044: Out-of-bounds Write (CWE-787)
value: Espruino 2v11.251 was discovered to contain a stack buffer overflow via src/jsvar.c in jsvNewFromString.
CWE description: The product writes data past the end, or before the beginning, of the intended buffer.
CVSS v3 base score: 7.8
Published: 2022-03-05T02:15Z
Repository: https://github.com/espruino/Espruino
Language: C
Files changed: ChangeLog, src/jsparse.c, src/jsparse.h, src/jsvar.c
Commit date: 2022-02-08 14:59:00 +0000
Commit message: Fix potential corruption issue caused by `delete [].__proto__` (fix #2142)

Diff: diff --git a/ChangeLog b/ChangeLog
index ce40279d6d..435274553f 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -7,10 +7,11 @@
             Memory usage improvement: Arrow functions only store value of 'this' if 'this' is used by code inside them (fix #2139)
             Add String.prototype.concat (fix #2140)
             Much-improved whitespace lexing code using single jumptable - 3% speed increase
             Allow `process.memory(false)` to get memory info but skip GC
             Graphics: fix height calculation bug in fillRect/clearRect when given an object with w/h as an argument
+            Fix potential corruption issue caused by `delete [].__proto__` (fix #2142)
 
      2v12 : nRF52840: Flow control XOFF is now sent at only 3/8th full - delays in BLE mean we can sometimes fill our 1k input buffer otherwise
             __FILE__ is now set correctly for apps (fixes 2v11 regression)
             Bangle.js: Ensure Bangle.getHealthStatus("day") resets at midnight (fix https://github.com/espruino/BangleApps/issues/1216)
             Bangle.js: Attempt to fix Bangle.buzz that occasionally doesn't stop
diff --git a/src/jsparse.c b/src/jsparse.c
index 64b48090d9..8e58641ad2 100644
--- a/src/jsparse.c
+++ b/src/jsparse.c
@@ -950,11 +950,16 @@ static NO_INLINE JsVar *jspGetNamedFieldInParents(JsVar *object, const char* nam
       jspEnsureIsPrototype(object, child);
       jsvUnLock(proto);
     } else if (strcmp(name, JSPARSE_INHERITS_VAR)==0) {
       const char *objName = jswGetBasicObjectName(object);
       if (objName) {
-        child = jspNewPrototype(objName);
+        JsVar *p = jsvSkipNameAndUnLock(jspNewPrototype(objName));
+        // jspNewPrototype returns a 'prototype' name that's already a child of eg. an array
+        // Create a new 'name' called __proto__ that links to it
+        JsVar *i = jsvNewFromString(JSPARSE_INHERITS_VAR);
+        if (p) child = jsvCreateNewChild(object, i, p);
+        jsvUnLock(i);
       }
     }
   }
 
   return child;
@@ -1370,12 +1375,17 @@ NO_INLINE JsVar *jspeFactorDelete() {
     if (jsvIsName(a) && !jsvIsNewChild(a)) {
       // if no parent, check in root?
       if (!parent && jsvIsChild(execInfo.root, a))
         parent = jsvLockAgain(execInfo.root);
 
-      if (jsvHasChildren(parent)) {
+#ifdef DEBUG
+      if (jsvHasChildren(parent)) assert(jsvIsChild(parent, a));
+#endif
+      if (jsvHasChildren(parent) && jsvIsChild(parent, a)) {
         // else remove properly.
+        /* we use jsvIsChild here just in case. delete probably isn't called
+        that often so it pays to be safe */
         if (jsvIsArray(parent)) {
           // For arrays, we must make sure we don't change the length
           JsVarInt l = jsvGetArrayLength(parent);
           jsvRemoveChild(parent, a);
           jsvSetArrayLength(parent, l, false);
@@ -2846,11 +2856,11 @@ JsVar *jspNewBuiltin(const char *instanceOf) {
   JsVar *objFunc = jswFindBuiltInFunction(0, instanceOf);
   if (!objFunc) return 0; // out of memory
   return objFunc;
 }
 
-/// Create a new Class of the given instance and return its prototype
+/// Create a new Class of the given instance and return its prototype (as a name 'prototype')
 NO_INLINE JsVar *jspNewPrototype(const char *instanceOf) {
   JsVar *objFuncName = jsvFindChildFromString(execInfo.root, instanceOf, true);
   if (!objFuncName) // out of memory
     return 0;
 
diff --git a/src/jsparse.h b/src/jsparse.h
index 80a765ef0f..71dde0a05e 100644
--- a/src/jsparse.h
+++ b/src/jsparse.h
@@ -35,11 +35,11 @@ JsVar *jspGetConstructor(JsVar *object);
 bool jspCheckStackPosition();
 
 /// Create a new built-in object that jswrapper can use to check for built-in functions
 JsVar *jspNewBuiltin(const char *name);
 
-/// Create a new Class of the given instance and return its prototype
+/// Create a new Class of the given instance and return its prototype (as a name 'prototype')
 NO_INLINE JsVar *jspNewPrototype(const char *instanceOf);
 
 /** Create a new object of the given instance and add it to root with name 'name'.
  * If name!=0, added to root with name, and the name is returned
  * If name==0, not added to root and Object itself returned */
diff --git a/src/jsvar.c b/src/jsvar.c
index 34d6e038ab..4935f597dd 100644
--- a/src/jsvar.c
+++ b/src/jsvar.c
@@ -2553,10 +2553,11 @@ void jsvAddName(JsVar *parent, JsVar *namedChild) {
         insertAfter = jsvLockSafe(prev);
       }
     }
 
     if (insertAfter) {
+      assert(jsvIsName(insertAfter));
       if (jsvGetNextSibling(insertAfter)) {
         // great, we're in the middle...
         JsVar *insertBefore = jsvLock(jsvGetNextSibling(insertAfter));
         jsvSetPrevSibling(insertBefore, jsvGetRef(namedChild));
         jsvSetNextSibling(namedChild, jsvGetRef(insertBefore));
@@ -2759,10 +2760,13 @@ JsVar *jsvFindChildFromVar(JsVar *parent, JsVar *childName, bool addIfNotFound)
 }
 
 void jsvRemoveChild(JsVar *parent, JsVar *child) {
   assert(jsvHasChildren(parent));
   assert(jsvIsName(child));
+#ifdef DEBUG
+  assert(jsvIsChild(parent, child));
+#endif
   JsVarRef childref = jsvGetRef(child);
   bool wasChild = false;
   // unlink from parent
   if (jsvGetFirstChild(parent) == childref) {
     jsvSetFirstChild(parent, jsvGetNextSibling(child));

Vulnerable code:         child = jspNewPrototype(objName);
      if (jsvHasChildren(parent)) {
/// Create a new Class of the given instance and return its prototype
/// Create a new Class of the given instance and return its prototype
Fixed code:             Fix potential corruption issue caused by `delete [].__proto__` (fix #2142)
        JsVar *p = jsvSkipNameAndUnLock(jspNewPrototype(objName));
        // jspNewPrototype returns a 'prototype' name that's already a child of eg. an array
        // Create a new 'name' called __proto__ that links to it
        JsVar *i = jsvNewFromString(JSPARSE_INHERITS_VAR);
        if (p) child = jsvCreateNewChild(object, i, p);
        jsvUnLock(i);
#ifdef DEBUG
      if (jsvHasChildren(parent)) assert(jsvIsChild(parent, a));
#endif
      if (jsvHasChildren(parent) && jsvIsChild(parent, a)) {
        /* we use jsvIsChild here just in case. delete probably isn't called
        that often so it pays to be safe */
/// Create a new Class of the given instance and return its prototype (as a name 'prototype')
/// Create a new Class of the given instance and return its prototype (as a name 'prototype')
      assert(jsvIsName(insertAfter));
#ifdef DEBUG
  assert(jsvIsChild(parent, child));
#endif