Search code examples
javascriptstatic-code-analysiscoveritysynopsis-detect

Cross side scripting vulnerability detected in javascript code


Please check the vulnerability on cross side scripting - "The untrusted data reaches a sink that may allow an attacker to control part of the response."

The property "req.body" is a source of untrusted data.
        let {
            userName,
…
                util.errorLoging({
                    "message": userName + ' ' + helper.statusMessage.NO_SCHEDULE.DATA + initialDate,
                    "bqJobId": bqJobId,
                    "userName": userName
                });
4. Passing "{"message" : userName + " " + helper.statusMessage.NO_SCHEDULE.DATA}" to "res.status(helper.statusCode.OK).send".
5. Calling "res.status(helper.statusCode.OK).send" with the tainted value in property "*" of "{"message" : userName + " " + helper.statusMessage.NO_SCHEDULE.DATA}". The untrusted data reaches a sink that may allow an attacker to control part of the response.
6. Escape non-constant data appropriately before concatenating it into HTML. The specific sequence of escapers necessary to make data safe depends on its syntactic position in the HTML. Allowing only safe characters (whitelisting) sometimes suffices to avoid XSS vulnerabilities, but only the strictest whitelists prevent all attacks.
                return res.status(helper.statusCode.OK).send({
2. Creating a tainted string using "userName".
3. Assigning a tainted string to "<storage from new>["message"]".
                    message: userName + ' ' + helper.statusMessage.NO_SCHEDULE.DATA,
                });

code

return res.status(helper.statusCode.OK).send({
                    message: userName + ' ' + helper.statusMessage.NO_SCHEDULE_DATA,
                });

Please let me know how this vulnerability can be corrected?

EDIT req.body contents

let { userName, initialDate, lastDate, bqJobId } = req.body


Solution

  • Understanding the finding

    The finding line:

    The property "req.body" is a source of untrusted data.
    

    is saying that req.body is untrusted data.

    I don't know if this is a direct quote from the tool or your summary, but let's take it at face value: the request comes from the network, so it is untrusted, also known as "tainted". That means an attacker could set it to any value they want, including things containing HTML scripts, for example:

    {
      "userName": "<script>alert('Gotcha!')</script>"
    }
    

    The code line:

    let { userName, initialDate, lastDate, bqJobId } = req.body
    

    copies the userName attribute of req.body into the userName local variable (via "destructuring assignment").

    The vulnerable code:

    return res.status(helper.statusCode.OK).send({
      message: userName + ' ' + helper.statusMessage.NO_SCHEDULE_DATA,
    });
    

    is diagnosed by the finding message:

    1. Calling res.status(helper.statusCode.OK).send with the tainted value in property * of {"message" : userName + " " + helper.statusMessage.NO_SCHEDULE.DATA}. The untrusted data reaches a sink that may allow an attacker to control part of the response.

    It explains that userName is concatenated into a string, and that string becomes the value of the message attribute of a new object.

    That new object is then passed to send. The tool says it flows from there into HTML output. (You haven't shown that part of the code, or specified what library is being used, so I'll just take that at face value as well.)

    When this happens, the HTML output will contain a string that is entirely under attacker control. With the example I gave above, an alert box will pop up, but it could be any malicious code.

    Incidentally, the phrase <storage from new> is due to the tool's C++ roots showing. In a JavaScript context, you should read that as <object literal>.

    Fixing the vulnerability

    The finding already explains the basic idea:

    1. Escape non-constant data appropriately before concatenating it into HTML. The specific sequence of escapers necessary to make data safe depends on its syntactic position in the HTML. Allowing only safe characters (whitelisting) sometimes suffices to avoid XSS vulnerabilities, but only the strictest whitelists prevent all attacks.

    The top answer to Can I escape HTML special chars in JavaScript? suggests this escaper:

    function escapeHtml(unsafe)
    {
        return unsafe
             .replace(/&/g, "&amp;")
             .replace(/</g, "&lt;")
             .replace(/>/g, "&gt;")
             .replace(/"/g, "&quot;")
             .replace(/'/g, "&#039;");
    }
    

    Assuming you add this function to your code, you should pass userName through it before concatenation:

    return res.status(helper.statusCode.OK).send({
      message: escapeHtml(userName) + ' ' + helper.statusMessage.NO_SCHEDULE_DATA,
      //       ^^^^^^^^^^
    });
    

    If you then test with the example input given above, you will see that the metacharacters in the payload have been neutralized:

    &lt;script&gt;alert(&#039;Gotcha!&#039;)&lt;/script&gt;
    

    and consequently the code is safe.