Fixing String conversion bug (#249)

alexbara2000 · web-flow · commit 94dcf255ebc9 · 2025-01-23T14:21:54.000+01:00
* Fixed taint propagation for String conversion fast path and utf8 strings
* Added integration tests
diff --git a/dom/base/nsJSUtils.h b/dom/base/nsJSUtils.h
@@ -171,6 +171,7 @@ inline bool AssignJSString(JSContext* cx, T& dest, JSString* s) {
 
   MOZ_ASSERT(read == JS::GetStringLength(s));
   handle.Finish(written, kAllowShrinking);
+  dest.AssignTaint(JS_GetStringTaint(s));
   return true;
 }
 
diff --git a/js/xpconnect/src/xpcpublic.h b/js/xpconnect/src/xpcpublic.h
@@ -430,6 +430,7 @@ class XPCStringConvert {
         //       nsStringBuffer::FromData takes void*.
         AssignFromStringBuffer(
             nsStringBuffer::FromData(const_cast<DestCharT*>(chars)), len, dest);
+        dest.AssignTaint(JS_GetStringTaint(s));
         return true;
       }
     } else if (callbacks == &sLiteralExternalString) {
@@ -442,6 +443,7 @@ class XPCStringConvert {
       // The characters represent a literal string constant
       // compiled into libxul; we can just use it as-is.
       dest.AssignLiteral(chars, len);
+      dest.AssignTaint(JS_GetStringTaint(s));
       return true;
     }
 
diff --git a/taint/test/mochitest/mochitest.ini b/taint/test/mochitest/mochitest.ini
@@ -42,6 +42,7 @@ support-files =
 [test_websocket.html]
 [test_push.html]
 [test_dom.html]
+[test_string_convertion_with_classes.html]
 scheme = https
 [test_url_object.html]
 [test_message_event.html]
diff --git a/taint/test/mochitest/test_string_convertion_with_classes.html b/taint/test/mochitest/test_string_convertion_with_classes.html
@@ -0,0 +1,65 @@
+<!DOCTYPE HTML>
+ <html>
+   <head>
+     <meta charset="utf-8">
+     <title>Test HTML Message Port Taint Sink</title>
+     <script src="/tests/SimpleTest/SimpleTest.js"></script>
+     <link rel="stylesheet" href="/tests/SimpleTest/test.css"/>
+     <script>
+      class SessionStorageManager {
+        constructor(storageKey) {
+            this.storageKey = storageKey;
+            this.storage = window.sessionStorage;
+            this.items = [];
+        }
+
+        get(key, factory) {
+            const newItem = factory(key);
+            this.items.push(newItem);
+            return newItem;
+        }
+
+        set() {
+            if (this.storage) {
+                var jsonString = JSON.stringify(this.items);
+                var encoded2=btoa(jsonString);
+                this.storage.setItem(this.storageKey, encoded2);
+                this.storage.setItem(this.storageKey, jsonString);
+            }
+        }
+      }
+
+      let string_content = "hello";
+      let number_of_tainted_flows = 4;
+      let i = 0;
+
+      SimpleTest.waitForExplicitFinish();
+      addEventListener("__taintreport", (report) => {
+          is(report.detail.str, string_content, "Check sink string content");
+          i += 1;
+          if (i >= number_of_tainted_flows) {
+            SimpleTest.finish();
+          }
+      }, false);
+
+      let taint_string = String.tainted(string_content);
+      const storageManager = new SessionStorageManager('myAppData');
+      function itemFactory(key) {
+          return { key, data: "thing" };
+      }
+
+      const item1 = storageManager.get('item1', itemFactory);
+      item1.data=[taint_string]
+      storageManager.set();
+      const item2 = storageManager.get('item2', itemFactory);
+      item2.data=[taint_string]
+      storageManager.set();
+     </script>
+   </head>
+   <body>
+     <p id="display"></p>
+     <div id="content" style="display: none"></div>
+     <p id="test"></p>
+     <button id="btn"></button>
+   </body>
+ </html>

Original file line number	Diff line number	Diff line change
`@@ -171,6 +171,7 @@ inline bool AssignJSString(JSContext* cx, T& dest, JSString* s) {`
`171`	`171`
`172`	`172`	`MOZ_ASSERT(read == JS::GetStringLength(s));`
`173`	`173`	`handle.Finish(written, kAllowShrinking);`
	`174`	`+ dest.AssignTaint(JS_GetStringTaint(s));`
`174`	`175`	`return true;`
`175`	`176`	`}`
`176`	`177`
Original file line number	Diff line number	Diff line change
`@@ -430,6 +430,7 @@ class XPCStringConvert {`
`430`	`430`	`// nsStringBuffer::FromData takes void*.`
`431`	`431`	`AssignFromStringBuffer(`
`432`	`432`	`nsStringBuffer::FromData(const_cast<DestCharT*>(chars)), len, dest);`
	`433`	`+ dest.AssignTaint(JS_GetStringTaint(s));`
`433`	`434`	`return true;`
`434`	`435`	`}`
`435`	`436`	`} else if (callbacks == &sLiteralExternalString) {`
`@@ -442,6 +443,7 @@ class XPCStringConvert {`
`442`	`443`	`// The characters represent a literal string constant`
`443`	`444`	`// compiled into libxul; we can just use it as-is.`
`444`	`445`	`dest.AssignLiteral(chars, len);`
	`446`	`+ dest.AssignTaint(JS_GetStringTaint(s));`
`445`	`447`	`return true;`
`446`	`448`	`}`
`447`	`449`