[prev in list] [next in list] [prev in thread] [next in thread] 

List:       oss-security
Subject:    Re: [oss-security] CVE request: MediaWiki 1.22.5 login csrf
From:       Jann Horn <jann () thejh ! net>
Date:       2014-03-29 13:57:25
Message-ID: 20140329135725.GC10345 () debjann ! fritz ! box
[Download RAW message or body]


On Sat, Mar 29, 2014 at 10:58:38AM +0000, Florent Daigniere wrote:
> On Sat, 2014-03-29 at 00:21 +0100, Jann Horn wrote:
> > However, this means that Login CSRF becomes a big security issue because it
> > would allow me to add evil JS to my account and then force the browser of
> > someone else to execute it in the context of the MediaWiki server's domain.
> 
> I had a look at how mediawiki generates its CSRF token... a smiley is
> worth a thousand words. :XD
> 
> -> includes/User.php:getEditToken
> 
> "Anon" users (whatever that is) share a token (EDIT_TOKEN_SUFFIX).
> Others have their pseudo-random "secret" hashed and stored in their
> session... and it's spit out using "return md5( $token . $salt ) .
> EDIT_TOKEN_SUFFIX;"
> 
> Few lines below is the function called matchEditToken(), *lazily*
> evaluating the above against what it receives on the wire.
> 
> I won't bore you with the details, but the above is very unlikely to be
> okay. In no particular order:
> -) according to the above, "Anon" users share the same CSRF tokens

Ah, that would probably allow an attacker to misuse his website's visitors
for spamming wikipedia, right? Not with any kind of elevated access though,
just coming from lots of different IPs and therefore hard to ban.


> -) the attacker can force a session (and its secret) onto a user: ever
> heard of http://www.php.net/manual/en/session.idpassing.php ? (grep
> tells me that neither session.use_only_cookies nor session.use_trans_sid
> are set)

Hmm, that sounds plausible, but I was unable to do that. Well, I'm not very
familiar with PHP, so I probably missed something.


> -) the way the tokens are generated and compared is not okay (lazy
> comparison in PHP, no constant time comparison, hash-length-extension
> attacks, ...)

Hm, yes, lazy comparison sounds bad, and non-constant-time also. But how
would you do a hash length extension against this? Wouldn't that only work
if the salt was in front of the token in the hash, not behind it?

["signature.asc" (application/pgp-signature)]

[prev in list] [next in list] [prev in thread] [next in thread]