createNewTainted flag for the AddSecureMarksToTaintedString method (#114

) * createNewTainted flag for the AddSecureMarksToTaintedString method * Update test/js/secure-marks.spec.js Co-authored-by: datadog-datadog-prod-us1[bot] <88084959+datadog-datadog-prod-us1[bot]@users.noreply.github.com> * lint --------- Co-authored-by: datadog-datadog-prod-us1[bot] <88084959+datadog-datadog-prod-us1[bot]@users.noreply.github.com>
DataDog · Jan 17, 2025 · 9a99ef1 · 9a99ef1
1 parent 69c07f2
commit 9a99ef1
Show file tree

Hide file tree

Showing 4 changed files with 80 additions and 14 deletions.
diff --git a/index.d.ts b/index.d.ts
@@ -28,7 +28,7 @@ declare module 'datadog-iast-taint-tracking' {
         createTransaction(transactionId: string): string;
         newTaintedString(transactionId: string, original: string, paramName: string, type: string): string;
         newTaintedObject(transactionId: string, original: any, paramName: string, type: string): any;
-        addSecureMarksToTaintedString(transactionId: string, taintedString: string, secureMarks: number): string;
+        addSecureMarksToTaintedString(transactionId: string, taintedString: string, secureMarks: number, createNewTainted?: boolean): string;
         isTainted(transactionId: string, ...args: string[]): boolean;
         getMetrics(transactionId: string, telemetryVerbosity: number): Metrics;
         getRanges(transactionId: string, original: string): NativeTaintedRange[];

diff --git a/src/api/taint_methods.cc b/src/api/taint_methods.cc
@@ -144,6 +144,7 @@ void AddSecureMarksToTaintedString(const FunctionCallbackInfo<Value>& args) {
     auto transactionIdArgument = args[0];
     auto taintedString = args[1];
     auto secureMarksArgument = args[2];
+    bool createNewTainted = args.Length() > 3 ? args[3]->BooleanValue(isolate) : true;
 
     args.GetReturnValue().Set(taintedString);
 
@@ -167,17 +168,25 @@ void AddSecureMarksToTaintedString(const FunctionCallbackInfo<Value>& args) {
     try {
         auto newRanges = transaction->GetSharedVectorRange();
         auto oRanges = taintedObj->getRanges();
-        taintedString = tainted::NewStringInstanceForNewTaintedObject
-                (isolate, v8::Local<v8::String>::Cast(taintedString));
-        for (auto it = oRanges->begin(); it != oRanges->end(); ++it) {
-            auto oRange = *it;
-            auto start = oRange->start;
-            auto end = oRange->end;
-            auto oSecureMarks = oRange->secureMarks;
-            newRanges->PushBack(transaction->GetRange(start, end, oRange->inputInfo, oSecureMarks | secureMarks));
+        if (createNewTainted) {
+            for (auto it = oRanges->begin(); it != oRanges->end(); ++it) {
+                auto oRange = *it;
+                auto start = oRange->start;
+                auto end = oRange->end;
+                auto oSecureMarks = oRange->secureMarks;
+                newRanges->PushBack(transaction->GetRange(start, end, oRange->inputInfo, oSecureMarks | secureMarks));
+            }
+            taintedString = tainted::NewStringInstanceForNewTaintedObject
+                    (isolate, v8::Local<v8::String>::Cast(taintedString));
+            transaction->AddTainted(utils::GetLocalPointer(taintedString), newRanges, taintedString);
+            args.GetReturnValue().Set(taintedString);
+        } else {
+            for (auto it = oRanges->begin(); it != oRanges->end(); ++it) {
+                auto oRange = *it;
+                auto oSecureMarks = oRange->secureMarks;
+                oRange->secureMarks = oSecureMarks | secureMarks;
+            }
         }
-        transaction->AddTainted(utils::GetLocalPointer(taintedString), newRanges, taintedString);
-        args.GetReturnValue().Set(taintedString);
     } catch (const std::bad_alloc& err) {
     } catch (const container::QueuedPoolBadAlloc& err) {
     } catch (const container::PoolBadAlloc& err) {

diff --git a/src/tainted/range.h b/src/tainted/range.h
@@ -11,7 +11,7 @@
 
 namespace iast {
 namespace tainted {
-using secure_marks_t = uint16_t;
+using secure_marks_t = uint32_t;
 class Range {
  public:
     explicit Range(int start, int end, InputInfo *inputInfo, secure_marks_t secureMarks);

diff --git a/test/js/secure-marks.spec.js b/test/js/secure-marks.spec.js
@@ -6,6 +6,25 @@
 
 const { TaintedUtils } = require('./util')
 const assert = require('assert')
+
+let referenceEqual
+function makeReferenceEqual () {
+  // eslint-disable-next-line no-new-func
+  referenceEqual = new Function('value1', 'value2', 'return %ReferenceEqual(value1, value2)')
+}
+
+try {
+  makeReferenceEqual()
+} catch (e) {
+  try {
+    const v8 = require('v8')
+    v8.setFlagsFromString('--allow-natives-syntax')
+    makeReferenceEqual()
+    v8.setFlagsFromString('--no-allow-natives-syntax')
+  // eslint-disable-next-line no-empty
+  } catch (e) { /* empty */ }
+}
+
 describe('Secure marks', function () {
   const id = TaintedUtils.createTransaction('666')
   const value = 'test'
@@ -117,11 +136,49 @@ describe('Secure marks', function () {
     const originalTaintedValue = TaintedUtils.newTaintedString(id, 'range1TOREPLACErange2', param, 'REQUEST')
     const taintedValueWithSecureMarks = TaintedUtils.addSecureMarksToTaintedString(id, originalTaintedValue, 0xffaf)
     const taintedValueWithSecureMarks2 =
-      TaintedUtils.addSecureMarksToTaintedString(id, originalTaintedValue, 0xffaff) // over the limit
+      TaintedUtils.addSecureMarksToTaintedString(id, originalTaintedValue, 0x1ffffffff) // over the limit
 
     const markedRanges1 = TaintedUtils.getRanges(id, taintedValueWithSecureMarks)
     const markedRanges2 = TaintedUtils.getRanges(id, taintedValueWithSecureMarks2)
     assert.equal(markedRanges1[0].secureMarks, 0xffaf)
-    assert.equal(markedRanges2[0].secureMarks, 0xfaff)
+    assert.equal(markedRanges2[0].secureMarks, 0xffffffff)
+  })
+
+  describe('createNewTainted flag', () => {
+    it('when false no new tainted should be created', () => {
+      const originalTaintedValue = TaintedUtils.newTaintedString(id, 'tainted', param, 'REQUEST')
+      const taintedWithSecureMarks = TaintedUtils.addSecureMarksToTaintedString(id, originalTaintedValue, 0b1, false)
+
+      assert(referenceEqual(originalTaintedValue, taintedWithSecureMarks))
+
+      const markedRanges1 = TaintedUtils.getRanges(id, originalTaintedValue)
+      const markedRanges2 = TaintedUtils.getRanges(id, taintedWithSecureMarks)
+      assert.equal(markedRanges1[0].secureMarks, 0b1)
+      assert.equal(markedRanges2[0].secureMarks, 0b1)
+    })
+
+    it('when undefined new tainted should be created', () => {
+      const originalTaintedValue = TaintedUtils.newTaintedString(id, 'tainted', param, 'REQUEST')
+      const taintedWithSecureMarks = TaintedUtils.addSecureMarksToTaintedString(id, originalTaintedValue, 0b1)
+
+      const markedRanges1 = TaintedUtils.getRanges(id, originalTaintedValue)
+      const markedRanges2 = TaintedUtils.getRanges(id, taintedWithSecureMarks)
+      assert.equal(markedRanges1[0].secureMarks, 0)
+      assert.equal(markedRanges2[0].secureMarks, 0b1)
+
+      assert(!referenceEqual(originalTaintedValue, taintedWithSecureMarks))
+    })
+
+    it('when true new tainted should be created', () => {
+      const originalTaintedValue = TaintedUtils.newTaintedString(id, 'tainted', param, 'REQUEST')
+      const taintedWithSecureMarks = TaintedUtils.addSecureMarksToTaintedString(id, originalTaintedValue, 0b1, true)
+
+      const markedRanges1 = TaintedUtils.getRanges(id, originalTaintedValue)
+      const markedRanges2 = TaintedUtils.getRanges(id, taintedWithSecureMarks)
+      assert.equal(markedRanges1[0].secureMarks, 0)
+      assert.equal(markedRanges2[0].secureMarks, 0b1)
+
+      assert(!referenceEqual(originalTaintedValue, taintedWithSecureMarks))
+    })
   })
 })