XSS in guestbook form
Permalink
If i post the following code (ignore the php open/close):In the 'name' or 'email' fields, there is a disastrous XSS vulnerability. Try it, it's a safe demo.
I 'fixed' it by adding some strip_tags() in concrete/blocks/guestbook/controller.php.
Is there some other place I should look?
>"></title></iframe></script></form></td></tr><br><iFraMe src=http://www.HackerSafe.com width=900 height=1100></IfRamE>
I 'fixed' it by adding some strip_tags() in concrete/blocks/guestbook/controller.php.
Is there some other place I should look?
Hey Andrew,
The form helper has the same XSS problems, it is not sanitizing vars before redisplaying them on a form that wasn't properly validated.
Which file are you making the fixes in? I would love to get my hand it when building forms with the form helper.
The form helper has the same XSS problems, it is not sanitizing vars before redisplaying them on a form that wasn't properly validated.
Which file are you making the fixes in? I would love to get my hand it when building forms with the form helper.
Exactly.
The sanitize will clean it up before it gets to the db, but if I enter an invalid string in either 'name' or 'email, it get displayed back to me with the validation error. At this point, I could be executing malicious code on your website.
Classic XSS.
I was able to fix it in my local /blocks/guestbook/controller.php, but I couldn't find a way to modify the 'validate' helper without doing it to the global (concrete) copy.
I assume that will get rewritten when I upgrade, and it will error out.
Is there something I'm missing?
The sanitize will clean it up before it gets to the db, but if I enter an invalid string in either 'name' or 'email, it get displayed back to me with the validation error. At this point, I could be executing malicious code on your website.
Classic XSS.
I was able to fix it in my local /blocks/guestbook/controller.php, but I couldn't find a way to modify the 'validate' helper without doing it to the global (concrete) copy.
I assume that will get rewritten when I upgrade, and it will error out.
Is there something I'm missing?
You could pull a copy of the guestbook block out into your local block scope (blocks/ instead of concrete/blocks) and make your modifications there. This would be safe from upgrades.
However, if you produce code that closes this vulnerability, I'm willing to be that it, or some permutation of it will wind up being applied to the guestbook block in core. Nobody likes a security hole :)
However, if you produce code that closes this vulnerability, I'm willing to be that it, or some permutation of it will wind up being applied to the guestbook block in core. Nobody likes a security hole :)
That's what I did for the guestbook change, but the script also uses the validation helper, which is not associated with a block.
I tried adding validation to the helpers folder higher up, but it errored out.
Didn't see the error, though, because my admin had error logging turned off. :/
Thanks to you and andrew for getting back to me on this, btw.
I tried adding validation to the helpers folder higher up, but it errored out.
Didn't see the error, though, because my admin had error logging turned off. :/
Thanks to you and andrew for getting back to me on this, btw.
I'm not sure I would make the modification at the validation helper level, although I could see adding an additional check to the validationhelper to error out on invalid tags.
I've added a fix for this to the guestbook block in subversion that will run strip tags on the request variables that are being shown to the user.
I've added a fix for this to the guestbook block in subversion that will run strip tags on the request variables that are being shown to the user.
Brilliant!
I added the tag handler in validation/strings.php
To keep my DB from getting all filled up with crap.
Our site gets checked by McAfee periodically, and they will hit a page thousands of times to try and find a vulnerability!
I added the tag handler in validation/strings.php
public function tags($field) { return !preg_match('/<[^<]+?>|<|>/', $field); }
To keep my DB from getting all filled up with crap.
Our site gets checked by McAfee periodically, and they will hit a page thousands of times to try and find a vulnerability!
In our current version we're running TextHelper::sanitize() on all these items, which strips tags (among other things.) The only time we're NOT running it is when displaying it after POST in the page. Is that what you mean? I'm fixing that in dev as we speak but I don't believe with concrete 5.4 (and possibly earlier) you can post a comment with this in it.