New Content Security Scan

[fronobulax]

No clue what label to use nor whether this is the best place to discuss.

We have a recent code scanning alert at https://github.com/kolmafia/kolmafia/security/code-scanning

The scanner wants to blame a PR that did not touch the file that is now flagged.

The issue is https://github.com/kolmafia/kolmafia/security/code-scanning/117 which can lead to a cross-site scripting vulnerability.

I think a fix could be simple but I am not especially fluent with JavaScript so I am not offering to make it. It may also be that the vulnerability cannot be exploited in mafia's environment.

Does someone want to fix it or disable this warning?

 

[Ryo_Sangnoir]

I'm not sure the warning is even correct. The warning says that DOM text is being interpreted as HTML. However, the line it highlights sets document.location.href, which is a string, and not HTML. If an attacker could control form fields on the page, they could use this script to send you to any page under adventure.php. But they could still do that if this warning was fixed. I think it's a false positive.

 

[Rinn]

You would need to look at the entire call path, getAdventureId() is using form data to return the string and that's where the warning is rooted from.

 

[Rinn]

But imo since this isn't really publicly accessible server app it probably doesn't really matter.