esotalk / esoTalk

Fat-free forum software.
GNU General Public License v2.0
1.47k stars 239 forks source link

esotalk topic [img] xss vulnerability #401

Closed evi1m0 closed 9 years ago

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

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

inliquid commented 9 years ago

can't reproduce

evi1m0 commented 9 years ago

http://esotalk.org/forum/1479-mass-email

Onmouseover image.

inliquid commented 9 years ago

Ok, works.

jgknight commented 9 years ago

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

inliquid commented 9 years ago

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

bogdanteodoru commented 9 years ago

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

$parsedUrl = parse_url($url);
if ($parsedUrl !== false) { /* valid! */ }
evi1m0 commented 9 years ago

htmlspecialchars

bogdanteodoru commented 9 years ago

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 commented 9 years ago

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.