Forum OpenACS Development: XSS vulnerability in XoWiki and a lot of other OpenACS pages

Hi everybody,

I just got an alert from a Brazilian government institution responsible for security saying that my website has a XSS vulnerability. I had already received other alerts concerning the same matter, but this time I have to do something or they will shut my website down. This institution is reponsible to check all government sites security, and if they are not happy, they can shut you down.

The problem is very simple: we basically don't check HTML code supplied as URL vars. An example is below:;b=alert;a(b(/XSS/.source));%3C/script%3E

If you follow this link, you are going to execute a JS code in a real URL domain. There are a lot of other places where the same behaviour can be seen, basically every single place where we give the possibility to receive HTML as URL vars. Another example is account-closed:;b=alert;a(b(/XSS/.source));%3C/script%3E

Of course, in order for this vulnerability do something harmfull, an user have to click on a link he/she receives in an e-mail, for instance, and we don't supply this kind of link. The problem is that user is clicking on a valid domain, and most users (almos every single user) will consider this as a valid URL.

I'm looking for a more general solution, like an RP filter, because I guess it would be insane to check every single page that receives HTML vars for this vulnerability. So, any suggestions and comments will be appreciated.


By default ad_page_contract does not allow HTML in query vars. You have to allow it specifically using the html or allhtml flags so this should be straightforward to deal with this.

Also not that you may have a misconfiguration of your allowedtag, or allowedattribute settings in ACS kernel which control the security of submitted HTML.

Hi Dave,

Thank you for your comment. In fact the problem only happens where you have the option to supply the HTML code as query vars in ad_page_contract, and that's exaclty what I'm talking about. These pages show this behaviour in general, and as they are a lot of pages, fix it for every single page would be insane.

However, you gave me a good hint: maybe we should change ad_page_contract to verify the HTML code? I did the test you said: changed the allowedtag parameter, but it seems like this check is only valid to form submition, not to URL vars. A possible fix would be to add this tag check to HTML URL vars?

yes, fix the filter.

I don't see any reason not to security check HTML. In generaal especiall for the account closed message, there is most likely a much better technique.

For account closed I suggest modifying it to use the util_user_message feature instead of passing the message in the URL.

Not sure about xowiki but I am not aware of a good reason to pass HTML in the way you have shown.

The only time you really need to pass HTML is during a form submission, where you are soliciting content from the user that may have HTML tags and it needs to be security checked.

That said, if you allow HTML to be passed on a form submit or just url variable it must be checked and we should do that by default for html input in ad_page_contract to reduce mistakes. Since normally form input is not processed by ad_page_contract this would cover both cases as long as you are using the form builder to process your forms.

Thanks for letting us know about this issue and your suggestions.

Eduardo, That should be doable without affecting anything. Coincidentally, I noticed that variables weren't checked last week while working on an alternate paradigm to ad_page_contract. iirc, ad_page_contract is where the vulnerability exists. I'll take a quick peek, see if I can identify the problem and suggest a solution.
I fixed the pages that call account closed to use the user message feature and committed the fix to HEAD.

No reason to pass the message around in that case anyway.

In ad_page_contract, the variable name is set to whatever is passed to ad_page_contract (via url) with no checking that it's a contiguous alphanumeric (plus dot) value:

# The name of the argument passed in the form
set actual_name [ns_set key $form $form_counter_i]

# The name of the formal argument in the page
set formal_name $actual_name

Fixing this, I believe will take care of the share of the problems you are referring to.

A glob based check, such as with "string match" would probably be faster than checking via regexp.


..for example:
string match -nocase {[a-z0-9_\.]*} $actual_name
How exactly does this affect the HTML content passed in the URL variables?

Is this a different problem you are addresses regarding varaible names? I thought they were confirmed against the ad_page_contract.

Hi Dave and Torben,

Dave, your patch doesn't include the util_user_message call. I've just included it, but it's not working somehow. Take a look at how my patch is right now:

Torben, I'm also working on ad-page-contract. I'll probably insert the same check as we have on the forms. As soon as it's working I'll post a patch.


The ad_returnredirect -message $message

has the same effect as calling util_user_message and it worked on a clean oacs-5-7 checkout.

Note your master template has to correctly inject the user message div. Not sure which template this is in , blank-master or default-master.

Dave, the ad_page_contract confirmation appears somewhat casual in its approach as attacks have become more sophisticated.

Even my previous example in this post is not adequate. The check for the form variables should actually be against the form submitted data before actual_name is assigned, to prevent some other nasty stuff that I can think about but don't want to post here.

ps. The check should bypass the rest of the loop if it fails.

Hi everybody,

More updates on this issue:

1 - AllowedTag is working for HTML code supplied as URL vars for ad_page_contact. I guess I had a cache problem. Sorry about that.

2 - Torben is right about our security in ad_html_security_check The checks done by this proc are good, but they don't answer to more sofisticated atack scenarios. I'm looking at some encoding tips to fix this matter:

3 - XoWiki doesn't run ad_html_security_check for supplied HTML code. That's where I'm working now, and if anybody can give me a clue about HTML parsing on XoWiki it would be helpfull.

Best regards and thank you for your comments.

The problem found on your site was most probably from the error message "page not found, do you want to create it new". There were versions of xowiki out there, which did not escape the string correctly. This was fixed in Sept 09, more than two years ago. What version of xowiki are you using?

I am currently on vacation high up in the Austrian alps. If there is more to do than upgrading and/or applying the patch i can still try to help.

Hi Gustaf,

Thank you for your reply. My XoWiki version was a little bit old because it was the last stable one I know (0.106.3) in the branch oacs-5-4. I ǘe tried two things then:

1 - Apply just the patch you've provided, but I got the following error:

::xowiki::Includelet html_encode not found

2 - Download HEAD version of XoWiki and XoTcl. With this version my old forms for news do not seem to work. I'm getting the following error:

[10/Feb/2011:23:16:44][28298.3056819088][-default:0-] Error: GET
referred by ""
::cr_folder1327: unable to dispatch method 'set_resolve_context'
while executing
"$page set_resolve_context -package_id [my id] -parent_id $parent_id"
(procedure "get_page_from_item_ref" line 56)
::4345 ::xowiki::Package->get_page_from_item_ref
invoked from within
"$package_id get_page_from_item_ref -use_package_path true -use_site_wide_pages true -use_prototype_pages true -parent_id [my parent_id] $page_name"

It seems like I have problem with upgrades, and it's always very difficult to upgrade without knowing the right procedures. I've also realized that the last stable version for XoWiki is 0.106.3, still with oacs-5-4 branch. With all the differences, I don't know wich version to install to minimize the errors, considering my websites have a lot of form pages and forms, mostly for news.

Can you give me any direction on this? If I can't solve this problem until tomorrow, they will cut my website down. Even migrating outside Wiki is difficult, because I don't see any other viable option to store the content.

Try making a procedure that does the work

ad_proc encode_html {string} {
Escape HTML code
} {
return [string map [list & "&amp;" < "&lt;" > "&gt;" \" "&quot;" ' "&#39;"] $string]

Then modify the patch to call this encode_html procedure instead of the xowiki includelet one and see if that helps.

Hi Dave,

Thank your for your answer, but I guess it won't work. It has to be called inside a method for xowiki::Weblog class, in an XoTcl perspective.

By the way, more info. I've just realized XoWiki upgrade failed because the name of some switches changed in form pages:

lead to error: unknown argument '-entries_of' for method 'instantiate_forms': valid arguments {-default_lang {}} {-parent_id {}} -forms:required -package_id:required
Details: unknown argument '-entries_of' for method 'instantiate_forms': valid arguments {-default_lang {}} {-parent_id {}} -forms:required -package_id:required
while executing
"::xotcl::interpretNonpositionalArgs $args"

The old switch to form entries was entries_of and it was changed to forms. I guess I would have to manually change all the news page I have in my system before I can upgrade. Is it right?

Dear Eduardo,

guess the error message comes from the prototype page "news". Actually, the upgrade scripts are supposed to add new revision to the instantiated prototype pages. In case, something went wrong, you can reload the prototype page via ds/shell (assuming the instance is called /xowiki, and the problem comes from the "news" prototype page

::xowiki::Package initialize -url /xowiki
$package_id import-prototype-page news

Hope, this helps

Eduardo, here is a backport of the patch to 0.106.*

This patch should work for you (i could not test it here). However, in general i would certainly recommend to upgrade; check, if you got an error during running the upgrade scripts (for every upgrade, you see a message "-- upgrading to" in the error log). Check, if upgrade worked correctly. I might be able to connect to the internet later this day.

ad_page_contract is now revised in head for the case I mentioned.

I've sent other possible cases needing review to Dave. Dave, let me know if you didn't get my replies to your email..



For the record, I changed "string match" with regexp, because string match isn't as flexible as I remember with the globs. change revised on head..
Hi Gustaf,

Thank you very much for all your help on this matter. I'm first trying to apply the patch you've provided to see if it solves the problem. My feeling is that it solves the security issue. I had to destroy the test server database because XoWiki upgrade failed, and it's going to take sometime to test it again.

If it works, I'm not sure if the upgrade is worthy, because it's always very difficult to work with HEAD version. Do you have any kind of CVS tag for a stable (or at least close to stable) version for XoWiki?