Skip to content
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

"The html tag check can be bypassed by obfuscating the html tag, leading to a false sense of security" #5

Closed
pppery opened this issue Oct 5, 2023 · 3 comments

Comments

@pppery
Copy link

pppery commented Oct 5, 2023

Per https://www.mediawiki.org/w/index.php?title=Extension:SaferHTMLTag&diff=prev&oldid=5789945

@pppery pppery changed the title "The html tag check can be bypassed by obfuscating the html tag, leading to a false sense of security "The html tag check can be bypassed by obfuscating the html tag, leading to a false sense of security" Oct 5, 2023
@bitsofmymind
Copy link
Member

I think I've fixed the issue. The page is parsed again before getting saved and if an HTML tag is detected by the Parser an error is triggered.

@bawolff
Copy link

bawolff commented Feb 20, 2024

There are still ways to bypass it. For example: {{#{{#ifexpr:{{#time:U}}+10>1708471495|tag:html|if:}}|<script>alert(1)</script>}}

I don't think its possible in principle to detect all possibilities in this manner. I think it is equivalent to the halting problem.


If you really want to do something like this, maybe the way to do it is during PST (i.e. ParserPreSaveTransformComplete hook), look for tags with a regex, capture the text inside the tag, calculate base64_encode(hash_hmac( 'sha256', $insideOfHtmlTag, $wgSecretKey ) ), and add that as an attribute to the tag only if the user is an authorized user. Then also override the html tag hook to calculate the same value, and only execute the html if the values match.

Still a bit sketchy. I'd worry about cases where PST is not stable (probably fixable by running PST twice and making sure the result is the same). But i think its more likely to be able to be made secure if running during PST instead of main parsing.

@bitsofmymind
Copy link
Member

Seems to me like the hash technique this could easily be bypassed. Unless I'm not understanding it correctly.

Right now, the code works to prevent pages containing the html tag from being saved. If indeed it is a halting type of problem then maybe we should figure a different solution entirely.

How about preventing a page from being parsed at display time if:

  1. an html tag is detected (easily done with a Parser hook)
  2. the last user to save the page does not have the rights to work with pages containing the html tag ?

Preventing such a page from even getting displayed could act as a last resort mesure to ensure the safety of the code (when all else has failed).

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

No branches or pull requests

3 participants