Ticket #363 (closed defect: fixed)

Opened 8 years ago

Last modified 8 years ago

XSS in UserSettings

Reported by: sakaru@… Owned by: DotMG
Priority: highest Milestone: 1.1.6.3
Component: actions Version: 1.1.6.2
Severity: critical Keywords: xss, usersettings, JS, vulnerability
Cc:

Description (last modified by JavaWoman) (diff)

PoC:  http://www.whiteacid.org/misc/xss_post_forwarder.php?xss_target=http://wiki.whiteacid.org/UserSettings&action=login&name="><script>alert('xss')</script>&password=&confpassword=&email=

When the login name is entered but no password is entered the user is sent back to the login form. What the user entered is then pasted back into the login name field. Insufficient filtering means JavaScript can be injected, such as injecting: "><script>alert('xss')</script> will create an alert box.

Change History

Changed 8 years ago by sakaru@…

I've fixed the flaw on my site so the old PoC no longer works. To fix it change line 498 in /actions/usersettings.php to: <td><input <?php echo $username_highlight; ?> name="name" size="40" value="<?php if (isset($_POSTname?)){ echo htmlentities($_POSTname?,ENT_QUOTES); }?>" /></td>

Working PoC:  http://www.whiteacid.org/misc/xss_post_forwarder.php?xss_target=http://wikkawiki.org/UserSettings&action=login&name=%22%3Easd&password=&confpassword=&email=

Changed 8 years ago by NilsLindenberg

  • keywords xss, usersettings added
  • priority changed from high to highest
  • component changed from unspecified to actions
  • severity changed from normal to critical
  • milestone set to 1.1.7

Changed 8 years ago by Amgine@…

Although this is a vulnerability, it is not a risk to the site or system. It is, however, a potential risk to referred users.

Changed 8 years ago by DarTar

I second Amgine's opinion - but many thanks for reporting this, sakaru. The fix will be included in the dev code.

Changed 8 years ago by DotMG

  • keywords usersettings, JS, vulnerability added; usersettings removed
  • owner changed from unassigned to DotMG
  • status changed from new to assigned

It can be a potential risk to the site or the system, but I won't explain why until this bug is fixed and version 1.1.7 released.

Anyway, it can lead to invalid xhtml.

Changed 8 years ago by DotMG

  • status changed from assigned to closed
  • resolution set to fixed

fixed in [204]

Changed 8 years ago by DotMG

Refixed in [205] : Previous changeset ([204]) made unable to connect if password contained special characters.

Changed 8 years ago by DarTar

Mahefa,

I see [205] didn't remove the new GetSafeVar() method you introduced in [204]. Is this made on purpose? When introducing a new core method, I suggest we share the announcement/discussion with other devs so we know how to use it.

Changed 8 years ago by DarTar

(bumping) Mahefa, see my last comment.

Changed 8 years ago by DotMG (not logged in)

OK! GetSafeVar() is just a way of doing

return ($this->htmlspecialchars_ent($_POST['var']));

with some checks if 'var' is transmitted via the correct method. Some internet hacks exploit the fact that you assume a certain parameter is transmitted at certain circumstances, and your code throws warnings (that reveal delicate information as the arborescence of your server) when you use strict error reporting and when these parameters are deliberately absent.

The method is still used in usersettings.php, but it doesn't just accomodate with password, and only with password.

Changed 8 years ago by JavaWoman

  • milestone changed from 1.1.7 to 1.1.6.3

changing milestone to 1.1.6.3 security release

Changed 8 years ago by JavaWoman

  • status changed from closed to reopened
  • resolution fixed deleted

reopening for 1.1.6.3 (note it's fixed in trunk!)

Changed 8 years ago by JavaWoman

  • status changed from reopened to closed
  • resolution set to fixed
  • description modified (diff)

Changesets [383 and [384] apply these changes to the 1.1.6.3 branch.

Closing the ticket again since it was reopened only for this branch

Note: See TracTickets for help on using tickets.