false positives: js/xss

[ktsaou]

Hi,

I have a few alerts about js/xss I can't understand how to work around them.

Here is an example:

https://lgtm.com/projects/g/netdata/netdata/alerts/?mode=list&rule=js%2Fxss

As you can see, I have escaped everything, but still LGTM complaints about XSS:

function escapeUserInputHTML(s) {
    return s.toString()
        .replace(/&/g, '&')
        .replace(/</g, '&lt;')
        .replace(/>/g, '&gt;')
        .replace(/"/g, '&quot;')
        .replace(/#/g, '&#35;')
        .replace(/'/g, '&#39;')
        .replace(/\(/g,'&#40;')
        .replace(/\)/g,'&#41;')
        .replace(/\//g,'&#47;');
}

function verifyURL(s) {
    if(typeof(s) === 'string' && (s.startsWith('http://') || s.startsWith('https://')))
        return s
            .replace(/'/g, '%22')
            .replace(/"/g, '%27')
            .replace(/\)/g, '%28')
            .replace(/\(/g, '%29');

    console.log('invalid URL detected:');
    console.log(s);
    return 'javascript:alert("invalid url");';
}

Any ideas?

Thank you!

[xiemaisi]

Thanks for your report and apologies for the delay in getting back to you. It looks like the URL in your original report doesn't work any more, but I'm assuming it refers to the same code as https://lgtm.com/projects/g/netdata/netdata/alerts/?mode=tree&ruleFocus=2022121412.

You are right that your escapeUserInputJS function makes it impossible for the user-provided string to break out of the attribute value, but it seems like a malicious user could still inject a javascript: URL into the href attribute. Or is that perhaps guarded against in some other way?

[ktsaou]

Hi,

Thank you for your reply.

I updated the URL and added a check to allow only URLs starting with http:// and https://.

However, LGTM still complains about XSS.

Thank you!

[xiemaisi]

Thanks for looking into this! At first sight this code now looks reasonably safe, we'll improve the analysis to recognise that and not flag it any longer. (In the meantime, you might consider adding a suppression comment.)

[xiemaisi]

Thank you again for reporting this and sorry for the delay in getting it addressed. This is now fixed, and the fix should be deployed next week.