@appland/scanner - npm Package Compare versions

Comparing version 1.68.0 to 1.69.0

built/rules/deserializationOfUntrustedData.js

@@ -6,42 +6,58 @@ "use strict";
 Object.defineProperty(exports, "__esModule", { value: true });
-const models_1 = require("@appland/models");
 const url_1 = require("url");
 const parseRuleDescription_1 = __importDefault(require("./lib/parseRuleDescription"));
-const precedingEvents_1 = __importDefault(require("./lib/precedingEvents"));
-const sanitizesData_1 = __importDefault(require("./lib/sanitizesData"));
-function allArgumentsSanitized(rootEvent, event) {
-    return (event.parameters || [])
-        .filter((parameter) => parameter.object_id)
-        .every((parameter) => {
-        for (const candidate of (0, precedingEvents_1.default)(rootEvent, event)) {
-            if ((0, sanitizesData_1.default)(candidate.event, parameter.object_id, DeserializeSanitize)) {
-                return true;
-            }
+const analyzeDataFlow_1 = __importDefault(require("./lib/analyzeDataFlow"));
+function valueHistory(value) {
+    const events = [];
+    const queue = [value];
+    for (;;) {
+        const current = queue.shift();
+        if (!current)
+            break;
+        const { origin, parents } = current;
+        if (!events.includes(origin))
+            events.push(origin);
+        queue.push(...parents);
+    }
+    return events;
+}
+function wasSanitized(value) {
+    return valueHistory(value).some(({ labels }) => labels.has(DeserializeSanitize));
+}
+function formatHistories(values) {
+    const histories = values.map(valueHistory);
+    return Object.fromEntries(histories.flatMap((history, input) => history.map((event, idx) => [`origin[${input}][${idx}]`, event])));
+}
+function label(name) {
+    return ({ labels }) => labels.has(name);
+}
+function matcher(startEvent) {
+    const flow = (0, analyzeDataFlow_1.default)([...(startEvent.message || [])], startEvent);
+    const results = [];
+    const sanitizedValues = new Set();
+    for (const [event, values] of flow) {
+        if (event.labels.has(DeserializeSanitize)) {
+            for (const v of values)
+                sanitizedValues.add(v);
+            continue;
         }
-        return false;
-    });
-}
-function build() {
-    function matcher(rootEvent) {
-        for (const event of new models_1.EventNavigator(rootEvent).descendants()) {
-            // events: //*[@authorization && truthy?(returnValue) && not(preceding::*[@authentication]) && not(descendant::*[@authentication])]
-            if (event.event.labels.has(DeserializeUnsafe) &&
-                !event.event.ancestors().find((ancestor) => ancestor.labels.has(DeserializeSafe))) {
-                if (allArgumentsSanitized(rootEvent, event.event)) {
-                    return;
-                }
-                else {
-                    return [
-                        {
-                            event: event.event,
-                            message: `${event.event} deserializes untrusted data`,
-                        },
-                    ];
-                }
+        if (!event.labels.has(DeserializeUnsafe))
+            continue;
+        const unsanitized = new Set(values.filter((v) => !(wasSanitized(v) || sanitizedValues.has(v))));
+        // remove any that have been passed into a safe deserialization function
+        for (const ancestor of event.ancestors().filter(label(DeserializeSafe))) {
+            for (const v of flow.get(ancestor) || []) {
+                unsanitized.delete(v);
             }
         }
+        const remaining = [...unsanitized];
+        if (remaining.length === 0)
+            continue;
+        results.push({
+            event: event,
+            message: `deserializes untrusted data: ${remaining.map(({ value: { value } }) => value)}`,
+            participatingEvents: formatHistories(remaining),
+        });
     }
-    return {
-        matcher,
-    };
+    return results;
 }
@@ -57,2 +73,3 @@ const DeserializeUnsafe = 'deserialize.unsafe';
     enumerateScope: false,
+    scope: 'http_server_request',
     references: {
@@ -64,3 +81,3 @@ 'CWE-502': new url_1.URL('https://cwe.mitre.org/data/definitions/502.html'),
     url: 'https://appland.com/docs/analysis/rules-reference.html#deserialization-of-untrusted-data',
-    build,
+    build: () => ({ matcher }),
 };

built/rules/lib/util.js

@@ -104,3 +104,3 @@ "use strict";
 function providesAuthentication(event, label) {
-    return event.returnValue && event.labels.has(label) && isTruthy(event.returnValue);
+    return !!event.returnValue && event.labels.has(label) && isTruthy(event.returnValue);
 }
@@ -107,0 +107,0 @@ exports.providesAuthentication = providesAuthentication;

CHANGELOG.md

@@ -0,1 +1,8 @@
+# [@appland/scanner-v1.69.0](https://github.com/applandinc/appmap-js/compare/@appland/scanner-v1.68.0...@appland/scanner-v1.69.0) (2022-08-23)
+
+
+### Features
+
+* Track specific untrusted data in unsafe deserialization rule ([d14fd4f](https://github.com/applandinc/appmap-js/commit/d14fd4f65fcbabfebdaf0d10dcae71dc563bc1fa))
+
 # [@appland/scanner-v1.68.0](https://github.com/applandinc/appmap-js/compare/@appland/scanner-v1.67.0...@appland/scanner-v1.68.0) (2022-08-19)
@@ -2,0 +9,0 @@

doc/rules/deserialization-of-untrusted-data.md

@@ -13,15 +13,32 @@ ---
   - deserialize.sanitize
+scope: http_server_request
 ---
 
 Finds occurrances of deserialization in which the mechanism employed is known to be unsafe, and the
-data is not known to be trusted.
+data comes from an untrusted source and hasn't passed through a sanitization mechanism.
 
 ### Rule logic
 
-Finds all events labeled `deserialize.unsafe`, that are not a descendant of an event labeled
-`deserialize.safe`. For each of these events, all event parameters are checked.
+Finds all events labeled `deserialize.unsafe` that receive tainted data (as
+determined by object identity or string value) as an input.
 
-Each parameter whose type is `string` or `object` is verified to ensure that it's trusted. For data
-to be trusted, it must be the return value of a function labeled `deserialize.sanitize`.
+For each of these events; checks if all the inputs have been sanitized.
 
+Data that has been passed to a function labeled `deserialize.sanitize` is
+assumed to be sanitized from this point onwards. Such a function could either
+check the value is sanitized (note no verification is currently done to ensure
+this result is checked) or return the transformed value after any necessary sanitization.
+
+Data passed to a function labeled `deserialized.safe` is considered in all
+functions called by it (down the callstack). Functions that first sanitize data
+and then use an unsafe deserialization function should carry this label.
+
+The set of tracked tainted data initially includes the HTTP message parameters
+and is expanded to include any non-primitive (ie. longer than 5 characters)
+observed outputs of functions that consume tainted data.
+
+The reliability of this rule now depends on completeness of the AppMap.
+If there is a data transformation that is	not captured it's invisible to the
+rule and will result in failure to associate it with the tracked untrusted data.
+
 ### Notes
@@ -34,6 +51,9 @@
 
-If you can guarantee that you are using unsafe deserialization in a safe way, but it's not possible
-to obtain the raw data from a function labeled `deserialize.sanitize`, you can wrap the
-deserialization in a function labeled `deserialize.safe`.
+Consider if the library you're using offers a safe deserialization function variant that you can
+use instead. Using unsafe functions is only rarely needed and typically requires a good reason.
 
+If you need to use the unsafe function, make sure you're able to handle unexpected input safely.
+Sanitize the data thoroughly first; label the sanitization function with `deserialize.sanitize` label
+or wrap the whole sanitization and deserialization logic in a function labeled `deserialize.safe`.
+
 If you need to deserialize untrusted data, JSON is often a good choice as it is only capable of
@@ -40,0 +60,0 @@ returning ‘primitive’ types such as strings, arrays, hashes, numbers and nil. If you need to

package.json

 {
   "name": "@appland/scanner",
-  "version": "1.68.0",
+  "version": "1.69.0",
   "description": "",
@@ -5,0 +5,0 @@ "bin": "built/cli.js",

No alert changes

Improved metrics

Total package byte prevSize

397378

increased by1.2%

Number of package files

202

increased by0.5%

Lines of code

6861

increased by1.43%

No dependency changes