Fix ranges in operations (#61)

---------

Co-authored-by: Carles Capell <[email protected]>

Author: uurien

src/api/substring.cc

@@ -60,7 +60,7 @@ void substring(const FunctionCallbackInfo<Value>& args) {
     int subjectLen = TO_V8STRING(subject)->Length();
     int start = TO_INTEGER_VALUE(args[3], context);
     int end = subjectLen;
-    if (argc > 4) {
+    if (argc > 4 && !args[4]->IsUndefined()) {
         end = TO_INTEGER_VALUE(args[4], context);
     }
 
@@ -130,8 +130,8 @@ void substr(const FunctionCallbackInfo<Value>& args) {
     auto subject = args[2];
     int subjectLen = TO_V8STRING(subject)->Length();
     int start = TO_INTEGER_VALUE(args[3], context);
-    int  length = subjectLen;
-    if (argc > 4) {
+    int length = subjectLen - start;
+    if (argc > 4 && !args[4]->IsUndefined()) {
         length = TO_INTEGER_VALUE(args[4], context);
     }
 

src/api/trim.cc

@@ -13,6 +13,8 @@
 #include "../tainted/transaction.h"
 #include "../iast.h"
 
+#define TO_V8STRING(arg) (v8::Local<v8::String>::Cast(arg))
+
 using v8::FunctionCallbackInfo;
 using v8::Value;
 using v8::Local;
@@ -65,9 +67,7 @@ void TaintTrimOperator(const FunctionCallbackInfo<Value>& args) {
             left++;
         } while (*selfCh++);
 
-        v8::String::Utf8Value resultStr(isolate, args[1]);
-        std::string cResultStr(*resultStr);
-        int resultLength = cResultStr.length();
+        int resultLength = TO_V8STRING(args[1])->Length();
 
         auto resultRanges = transaction->GetSharedVectorRange();
         auto end = ranges->end();
@@ -136,9 +136,7 @@ void TaintTrimEndOperator(const FunctionCallbackInfo<Value>& args) {
             return;
         }
 
-        v8::String::Utf8Value resultStr(isolate, args[1]);
-        std::string cResultStr(*resultStr);
-        int resultLength = cResultStr.length();
+        int resultLength = TO_V8STRING(args[1])->Length();
 
         auto resultRanges = transaction->GetSharedVectorRange();
         auto end = ranges->end();

src/utils/propagation.cc

@@ -17,6 +17,7 @@ SharedRanges* getRangesInSlice(Transaction* transaction, TaintedObject* obj, int
         return newRanges;
     }
 
+    int resultLength = sliceEnd - sliceStart;
     for (auto it = oRanges->begin(); it != oRanges->end(); ++it) {
         auto oRange = *it;
         int start, end;
@@ -49,6 +50,10 @@ SharedRanges* getRangesInSlice(Transaction* transaction, TaintedObject* obj, int
             end = sliceEnd;
         }
 
+        if (end > resultLength) {
+            end = resultLength;
+        }
+
         if (!newRanges) {
             newRanges = transaction->GetSharedVectorRange();
         }

test/js/substr.spec.js

@@ -25,6 +25,17 @@ const rangesTestCases = [
     start: 0,
     end: 3
   },
+  {
+    source: ':+-foobarbaz-+:',
+    result: ':+-barbaz-+:',
+    start: 3,
+    end: undefined
+  },
+  {
+    source: ':+-foobarbaz-+:',
+    result: ':+-barbaz-+:',
+    start: 3
+  },
   {
     source: ':+-foobarbaz-+:',
     result: ':+-bar-+:',
@@ -113,6 +124,17 @@ const rangesTestCases = [
     result: 'b:+-a-+:z:+-z-+::+-z-+:',
     start: 6,
     end: 20
+  },
+  {
+    source: 'foo:+-bar-+:b:+-a-+:z:+-z-+::+-z-+:',
+    result: 'b:+-a-+:z:+-z-+::+-z-+:',
+    start: 6,
+    end: undefined
+  },
+  {
+    source: 'foo:+-bar-+:b:+-a-+:z:+-z-+::+-z-+:',
+    result: 'b:+-a-+:z:+-z-+::+-z-+:',
+    start: 6
   }
 ]
 
@@ -227,13 +249,17 @@ describe('Check Ranges format', function () {
     TaintedUtils.removeTransaction(id)
   })
 
-  rangesTestCases.forEach(({ source, start, end, result }) => {
+  rangesTestCases.forEach((testData) => {
+    const { source, start, end, result } = testData
+
     it(`Test ${source}`, function () {
       const inputString = taintFormattedString(id, source)
       assert.equal(TaintedUtils.isTainted(id, inputString), true, 'Not tainted')
       const res = inputString.substr(start, end)
 
-      const ret = TaintedUtils.substr(id, res, inputString, start, end)
+      const ret = testData.hasOwnProperty('end')
+        ? TaintedUtils.substr(id, res, inputString, start, end)
+        : TaintedUtils.substr(id, res, inputString, start)
       assert.equal(res, ret, 'Unexpected value')
 
       const formattedResult = formatTaintedValue(id, ret)

test/js/substring.spec.js

@@ -19,6 +19,17 @@ const rangesTestCases = [
     start: -6,
     end: -3
   },
+  {
+    source: ':+-foobarbaz-+:',
+    result: ':+-barbaz-+:',
+    start: 3,
+    end: undefined
+  },
+  {
+    source: ':+-foobarbaz-+:',
+    result: ':+-barbaz-+:',
+    start: 3
+  },
   {
     source: ':+-foobarbaz-+:',
     result: ':+-foo-+:',
@@ -78,6 +89,17 @@ const rangesTestCases = [
     start: 6,
     end: 9
   },
+  {
+    source: 'foo:+-bar-+:baz',
+    result: ':+-r-+:baz',
+    start: 5,
+    end: undefined
+  },
+  {
+    source: 'foo:+-bar-+:baz',
+    result: ':+-r-+:baz',
+    start: 5
+  },
   {
     source: 'foo:+-bar-+:b:+-az-+:',
     result: 'b:+-az-+:',
@@ -227,13 +249,16 @@ describe('Check Ranges format', function () {
     TaintedUtils.removeTransaction(id)
   })
 
-  rangesTestCases.forEach(({ source, start, end, result }) => {
+  rangesTestCases.forEach((testData) => {
+    const { source, start, end, result } = testData
     it(`Test ${source}`, function () {
       const inputString = taintFormattedString(id, source)
       assert.equal(TaintedUtils.isTainted(id, inputString), true, 'Not tainted')
       const res = inputString.substring(start, end)
 
-      const ret = TaintedUtils.substring(id, res, inputString, start, end)
+      const ret = testData.hasOwnProperty('end')
+        ? TaintedUtils.substring(id, res, inputString, start, end)
+        : TaintedUtils.substring(id, res, inputString, start)
       assert.equal(res, ret, 'Unexpected value')
 
       const formattedResult = formatTaintedValue(id, ret)

test/js/util.js

@@ -47,10 +47,16 @@ function formatTaintedValue (transactionId, taintedValue) {
     return taintedValue
   }
   const ranges = TaintedUtils.getRanges(transactionId, taintedValue)
+
   if (!ranges || ranges.length === 0) {
     return taintedValue
   }
   checkRangesOrder(ranges)
+
+  if (ranges[ranges.length - 1].end > taintedValue.length) {
+    throw new Error(`Ranges out of value: max = ${taintedValue.length} and current = ${ranges[ranges.length - 1].end}`)
+  }
+
   return ranges.reduce((formattedString, range) => {
     formattedString =
       formattedString.slice(0, range.start + offset) +