Cross Site scripting holes in Formbuilder

[bob_basli]

Someone has been testing my site http://www.shotsaver.nl and through the formbuilder he send me this message:

there are several Cross Site scripting holes in Formbuilder. You must change the standard templates. I'd advice strongly against displaying any user input variables on the submit template.
Example: try the following string in 'Reactie'
<img src=http://kkc-it.de/1.gif
onload=alert(navigator.userAgent)//
Best regards,

Does anyone know what he means and how I can solve it?

[manuel]

Hi bob_basli,

I guess he means you should "validate" the user input to convert HTML code to plain text. (ex: convert "<" and ">" to "<" and ">")

You should be able to do this by using a "Validation UDT" (it's not very documented but you can find references to this in the module help)

I haven't tested this myself so i can't give you any concrete examples...

cfr: http://php.net/manual/en/function.htmlspecialchars.php

Greetings,
Manuel

[Duketown]

bob_basli,

I tested it in a test site. Had also problem with cross scripting.
Based upon manuels remark I prepared the following code for the UDT:

Code: Select all

// To prevent cross scripting
// ie. "<a href='test'>Test</a>" will become <a href='test'>Test</a>

$input = $params[0];
$new = htmlspecialchars($input, ENT_QUOTES);
echo $new;

Next step would be to include it in the form that is in place. However the version in use (5.11) of formbuilder is not capable of validating via UDT. Looking at the about of the module in a 1.10 environment, it seems that version 0.6.1 and up of formbuilder make it possible (though I'm not sure about this).
Tab Form Submission should contain the validation UDT, but not sure if this is at to high level.

Thanks for informing the community about this.

Duketown

[sjg]

Fixed in svn.

Edit action.default.php, and in line 156, change:

Code: Select all

$aeform->setFinishedFormSmarty();

to

Code: Select all

$aeform->setFinishedFormSmarty(true);

This will run htmlspecialchars on all submitted values (but will preserve the <br> tag).

[manuel]

Dear sjg,

There is a small typo in the post above, it should be "Form" and not "From".
Thx for this quick (and permanent!) solution.

Code: Select all

$aeform->setFinishedFormSmarty(true);

Greetings,
Manuel

[bob_basli]

so getting the latest from svn and fixing the typo will be all I need to do to permanenly fix this?

[manuel]

Dear bob_basli,

What i mean with "permanent" is that with the next FormBuilder release this change will be included for all users.

This would be a rare case where you could manually edit the action.default.php FormBuilder file (add "true" in the right location) and not don't have to worry about re-doing the changes when upgrading the module.

The typo is only concerning the forum post, i don't believe there's a typo in the FormBuilder code... (i haven't tested this solution yet though).
I was checking the code in the action.default.php file and it read "Form", not "From". (it's still correct in the SVN )

Greetings,
Manuel

[manuel]

ps: @sjg - I'm enjoying your book! I even took it with me to the beach

[bob_basli]

Thanks Manuel,

So just the action.default.php from the svn is sufficient for now?

[RonnyK]

bob,

changing the single line in your code will be enough. It is not possible to say if other changes in the action.default.php will cause issues. I would just modify the single line in the available .php and then upgrade when new version is cut.

Ronny