Javascript login form includes unencoded parameters

Bug #1009784 reported by Richard Mansfield
262
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Mahara
Fix Released
Critical
Son Nguyen

Bug Description

Discovered by Emanuel Bronshtein, present in all versions.

 parameters from URI are passed to javascript innerHTML without proper encoding, it possible to use some encoding inside javascript strings, as:
 \x22\x3E = ">
 \u0022\u003E = ">
 by using this encoding it possible to trigger XSS.

 Payload:
 "><h1>XSS</h1><img src=9 onerror=alert("XSS")> = \x22\x3E\x3C\x68\x31\x3E\x58\x53\x53\x3C\x2F\x68\x31\x3E\x3C\x69\x6D\x67\x20\x73\x72\x63\x3D\x39\x20\x6F\x6E\x65\x72\x72\x6F\x72\x3D\x61\x6C\x65\x72\x74\x28\x22\x58\x53\x53\x22\x29\x3E

 XSS Example:
 http://localhost/mahara-1.5.1/mahara-1.5.1/htdocs/admin/users/changeuser.php?xss=\x22\x3E\x3C\x68\x31\x3E\x58\x53\x53\x3C\x2F\x68\x31\x3E\x3C\x69\x6D\x67\x20\x73\x72\x63\x3D\x39\x20\x6F\x6E\x65\x72\x72\x6F\x72\x3D\x61\x6C\x65\x72\x74\x28\x22\x58\x53\x53\x22\x29\x3E
 http://localhost/mahara-1.5.1/mahara-1.5.1/htdocs/admin/users/bulk.php?xss=\x22\x3E\x3C\x68\x31\x3E\x58\x53\x53\x3C\x2F\x68\x31\x3E\x3C\x69\x6D\x67\x20\x73\x72\x63\x3D\x39\x20\x6F\x6E\x65\x72\x72\x6F\x72\x3D\x61\x6C\x65\x72\x74\x28\x22\x58\x53\x53\x22\x29\x3E
---

When a logged-out user tries to access a page that requires them to be logged in, a login form is generated, all get parameters are added to the form's action attribute & all post parameters are added as hidden elements inside the form. The login form is then inserted into the dom using innerHTML. This is for convenience, to let the user continue whatever it was they were trying to do if, for example, their session expired. I think it may be enough to url encode each query parameter name & value in the action url before generating the form, but haven't tested this yet.

Revision history for this message
Richard Mansfield (richard-mansfield) wrote :

This appears to fix it for me, but I'd appreciate some more opinions.

I think we also need to have a good look through all the innerHTML calls, because this is not necessarily restricted to the login form. I think 'Pieform' forms with jsform = true are doing the right thing though.

Revision history for this message
Richard Mansfield (richard-mansfield) wrote :

The same problem exists in versions 1.2, 1.4, 1.5, and the same patch fixes it.

[On 1.2 & 1.4, that patch won't apply cleanly with plain 'git am' or 'git apply', but 'patch' works, as do 'git am -3' and 'git apply -C2']

Son Nguyen (ngson2000)
Changed in mahara:
assignee: nobody → Son Nguyen (ngson2000)
status: Confirmed → In Progress
Revision history for this message
Son Nguyen (ngson2000) wrote :

There are
 - 66 innerHTML calls in Mahara PHP code(*.php).
 - 10 calls in templates (*.tpl)
 - 40 calls in Mahara JS code(*.js)

and over 500 calls in other third party packages including jquery, jscalendar, jscolor, Mochikit, tinymce, and flowplayer.

I would search these calls to identify XSS vulnerabilities.

Revision history for this message
Son Nguyen (ngson2000) wrote :

Identification XSS vulnerabilities using innerHTML calls

1. PHP code
  - admin/extensions/plugins.php -> OK
  - admin/upgrade.php -> OK
  - artefact/blog/view -> OK
  - artefact/internal/blocktype/textbox -> OK

Revision history for this message
Son Nguyen (ngson2000) wrote :

A report and patch have been sent to Melissa for verifying and reviewing.

Revision history for this message
Ruslan Kabalin (rkabalin) wrote :

The patch looks good to me.

Revision history for this message
Melissa Draper (melissa) wrote :
security vulnerability: yes → no
visibility: private → public
Changed in mahara:
status: In Progress → Fix Released
Melissa Draper (melissa)
security vulnerability: no → yes
Revision history for this message
Mahara Bot (dev-mahara) wrote : A change has been merged

Reviewed: https://reviews.mahara.org/1459
Committed: http://gitorious.org/mahara/mahara/commit/c24ccb84d79a5032cf58adaed401fd6399018d95
Submitter: Hugh Davenport (<email address hidden>)
Branch: master

commit c24ccb84d79a5032cf58adaed401fd6399018d95
Author: Richard Mansfield <email address hidden>
Date: Fri Jun 8 11:29:16 2012 +1200

    Json-encode login form when injected by js (bug #1009784)

    Change-Id: Ia81053332cfa9e0f79268031795af8d34b45ff78
    Signed-off-by: Richard Mansfield <email address hidden>

To post a comment you must log in.
This report contains Public Security information  
Everyone can see this security related information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.