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

esotalk topic [img] xss vulnerability #401

Closed
evi1m0 opened this Issue Dec 26, 2014 · 9 comments

Comments

Projects
None yet
5 participants
@evi1m0

evi1m0 commented Dec 26, 2014

[img][url=http://onclick=alert(document.cookie)//.com]http://www.hackersoul.com/image.jpg[/url][/img]

625accb7-10b3-417e-b663-103bcef5bec6

@inliquid

This comment has been minimized.

Show comment
Hide comment
@inliquid

inliquid Dec 26, 2014

Contributor

can't reproduce

Contributor

inliquid commented Dec 26, 2014

can't reproduce

@evi1m0

This comment has been minimized.

Show comment
Hide comment
@inliquid

This comment has been minimized.

Show comment
Hide comment
@inliquid

inliquid Dec 27, 2014

Contributor

Ok, works.

Contributor

inliquid commented Dec 27, 2014

Ok, works.

@jgknight jgknight added the Bug label Jan 5, 2015

@jgknight

This comment has been minimized.

Show comment
Hide comment
@jgknight

jgknight Jan 5, 2015

Contributor

Nice find, we'll have to figure out the best solution to prevent this.

Contributor

jgknight commented Jan 5, 2015

Nice find, we'll have to figure out the best solution to prevent this.

@inliquid

This comment has been minimized.

Show comment
Hide comment
@inliquid

inliquid Mar 15, 2015

Contributor

Guys, any chance that it's going to be fixed? I remind it's a security hole.

Contributor

inliquid commented Mar 15, 2015

Guys, any chance that it's going to be fixed? I remind it's a security hole.

@tobscure tobscure added the Critical label Mar 15, 2015

@bogdanteodoru

This comment has been minimized.

Show comment
Hide comment
@bogdanteodoru

bogdanteodoru Mar 16, 2015

Maybe use the PHP parse_url function? Simplest solution that comes to mind.

$parsedUrl = parse_url($url);
if ($parsedUrl !== false) { /* valid! */ }

Maybe use the PHP parse_url function? Simplest solution that comes to mind.

$parsedUrl = parse_url($url);
if ($parsedUrl !== false) { /* valid! */ }
@evi1m0

This comment has been minimized.

Show comment
Hide comment
@evi1m0

evi1m0 Mar 16, 2015

htmlspecialchars

evi1m0 commented Mar 16, 2015

htmlspecialchars

@bogdanteodoru

This comment has been minimized.

Show comment
Hide comment
@bogdanteodoru

bogdanteodoru Mar 16, 2015

Or

$str = mb_convert_encoding($str, 'UTF-8', 'UTF-8');
$str = htmlentities($str, ENT_QUOTES, 'UTF-8');

See http://stackoverflow.com/questions/110575/do-htmlspecialchars-and-mysql-real-escape-string-keep-my-php-code-safe-from-inje

Or

$str = mb_convert_encoding($str, 'UTF-8', 'UTF-8');
$str = htmlentities($str, ENT_QUOTES, 'UTF-8');

See http://stackoverflow.com/questions/110575/do-htmlspecialchars-and-mysql-real-escape-string-keep-my-php-code-safe-from-inje

@jgknight

This comment has been minimized.

Show comment
Hide comment
@jgknight

jgknight Mar 27, 2015

Contributor

One problem with proposed solutions is, as far as I can tell, onclick=alert(document.cookie)// are all valid characters within a URL. Filtering out any of those would potentially break valid URLs.

From glancing over the function, it seems like the simple way to solve this is to reorder the processing of BBCode so that images are processed before URLs. Then modify the regex of the [img] tag to check for valid protocols such as http/https at the beginning. I think this would prevent nesting of [img] tags, and then the URL parser would only run on URLs that are not wrapped in html.

I will do some testing on some solutions for this. I think the better way to solve this would be to replace the plugin with some 3rd party BBCode parser but that probably won't happen.

Contributor

jgknight commented Mar 27, 2015

One problem with proposed solutions is, as far as I can tell, onclick=alert(document.cookie)// are all valid characters within a URL. Filtering out any of those would potentially break valid URLs.

From glancing over the function, it seems like the simple way to solve this is to reorder the processing of BBCode so that images are processed before URLs. Then modify the regex of the [img] tag to check for valid protocols such as http/https at the beginning. I think this would prevent nesting of [img] tags, and then the URL parser would only run on URLs that are not wrapped in html.

I will do some testing on some solutions for this. I think the better way to solve this would be to replace the plugin with some 3rd party BBCode parser but that probably won't happen.

jgknight added a commit to jgknight/esoTalk that referenced this issue Apr 6, 2015

Fix XSS in BBcode, issue #401
This fixes #401 where the regex/order of BBcode allows injection of
javascript, introducing the possibility of Cross Site Scripting (XSS)
attacks.

@jgknight jgknight self-assigned this Apr 6, 2015

@jgknight jgknight closed this in #424 Apr 7, 2015

jgknight added a commit that referenced this issue Apr 7, 2015

xmulym added a commit to xmulym/esoTalk that referenced this issue May 5, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment