Skip to content

Vulnerability shows up/miss because of wrapping by (javascript) blockstatement #18652

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Anemone95 opened this issue Feb 2, 2025 · 3 comments
Labels
question Further information is requested

Comments

@Anemone95
Copy link

Here is the code(sorry I spent hours reducing the code but failed):

{
    function n(t, e) {
        var r = "undefined" != typeof Symbol && t[Symbol.iterator] || t["@@iterator"];
        if (!r) {
            if (Array.isArray(t) || (r = function (t, e) {
                if (t) {
                    if ("string" == typeof t) return o(t, e);
                    var r = {}.toString.call(t).slice(8, -1);
                    "Object" === r && t.constructor && (r = t.constructor.name);
                    return "Map" === r || "Set" === r ? Array.from(t) : "Arguments" === r || /^(?:Ui|I)nt(?:8|16|32)(?:Clamped)?Array$/.test(r) ? o(t, e) : void 0;
                }
            }(t)) || e && t && "number" == typeof t.length) {
                r && (t = r);
                var n = 0;
                var i = function () {
                };
                return {
                    s: i,
                    n: function () {
                        return n >= t.length ? {
                            done: !0
                        } : {
                            done: !1,
                            value: t[n++]
                        };
                    },
                    e: function (t) {
                        throw t;
                    },
                    f: i
                };
            }
        }
        var a;
        var s = !0;
        var c = !1;
        return {
            s: function () {
                r = r.call(t);
            },
            n: function () {
                var t = r.next();
                s = t.done;
                return t;
            },
            e: function (t) {
                c = !0;
                a = t;
            },
            f: function () {
                try {
                    s || null == r.return || r.return();
                } finally {
                    if (c) throw a;
                }
            }
        };
    }

    function o(t, e) {
        (null == e || e > t.length) && (e = t.length);
        for (var r = 0, n = Array(e); r < e; r++) n[r] = t[r];
        return n;
    }

    exports.KV = A;
    var i;
    var a = "undefined" != typeof process && process.env && process.env.DEBUG || void 0;
    var s = [];
    var c = [];
    var u = [];
    a && d(a);

    function d(t) {
        i = t;
        s = [];
        c = [];
        var e;
        var r = /\*/g;
        var o = t.split(",").map(function (t) {
            return t.trim().replace(r, ".*?");
        });
        var a = n(o);
        e = a.n()
        var l = e.value;
        new RegExp("^".concat(l.substr(1), "$"));
    }
}

If I directly build a database and scan that, I get nothing. But if I comment out the first line and last line, I get a vulnerability:

"Regular expression injection","User input should not be used in regular expressions without first being escaped, otherwise a malicious user may be able to inject an expression that could require exponential time on certain inputs.","error","This regular expression is constructed from a [[""environment variable""|""relative:///52181.js:68:61:68:71""]].","/52181.js","86","20","86","47"

The way I run codeql is like

$TARGET_DIR="./test"
codeql database create --language=javascript codeql-database --source-root="$TARGET_DIR" --overwrite
codeql database analyze ./codeql-database/ /codeql/codeql-repo/javascript/ql/src/Security/CWE-730/RegExpInjection.ql --format=csv --output="$OUTPUT_FILE" --threads=10
@redsun82
Copy link
Contributor

redsun82 commented Feb 4, 2025

👋 @Anemone95 Thanks for reaching out with this!

It seems like the semantics of function declarations within blocks is not that clear cut and may vary depending on the engine implementation (see some discussion here), although while some of that might be non-standard, it could be considered de facto standard based on the most common engines.

That said, your issue is prompting us to accommodate this use case (see #18661, currently work in progress), so a future release might deal with this case in a more appropriate way. So thanks again for this! 🙌

@asgerf
Copy link
Contributor

asgerf commented Feb 6, 2025

Hi @Anemone95,

Thanks again for reporting this and especially for reducing it to a small reproduction case. This helped a lot.

The fix has been merged (#18661) and should be in a release in about two weeks.

@asgerf asgerf closed this as completed Feb 6, 2025
@Anemone95
Copy link
Author

Perfect! Thanks for your help also!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

3 participants