api.php?action=unblock returns the full user object
Closed, ResolvedPublic

Description

When you unblock someone via the API, you get the full user object back. This includes mPassword (hashed, so I'm not sure how much of a big deal this is), mToken, mEmail (not public info), and some other things.

On a slightly unrelated note, I'm not sure api.php?meta=userinfo should return the watchlisttoken preference... In the UI it says 'Filling in this field with a secret key will generate an RSS feed for your watchlist. Anyone who knows the key in this field will be able to read your watchlist, so choose a secure value.'


Version: 1.20.x
Severity: normal

bzimport added a subscriber: Unknown Object (MLST).
bzimport set Reference to bz43518.
Krenair created this task.Via LegacyDec 30 2012, 1:13 PM
Krenair added a comment.Via ConduitDec 30 2012, 1:58 PM

(In reply to comment #0)

On a slightly unrelated note, I'm not sure api.php?meta=userinfo should
return
the watchlisttoken preference... In the UI it says 'Filling in this field
with
a secret key will generate an RSS feed for your watchlist. Anyone who knows
the
key in this field will be able to read your watchlist, so choose a secure
value.'

Ignore this bit. meta=userinfo is for the current user only.

Krenair added a comment.Via ConduitDec 30 2012, 2:05 PM

Created attachment 11567
Patch

attachment 0001-bug-43518-API-action-unblock-should-return-the-user-.patch ignored as obsolete

Krenair added a comment.Via ConduitDec 30 2012, 2:06 PM

This was caused by r83855.

demon added a comment.Via ConduitDec 31 2012, 12:57 PM

Interesting that __toString() doesn't seem to be working.

Krenair added a comment.Via ConduitDec 31 2012, 3:52 PM

The API uses json_encode (When using format=json, obviously. This bug probably doesn't apply to other formats), which doesn't appear to use __toString() (Why represent it as a string when we know about objects?).


<?php
class A {
public $youshouldnotseethis = "uh oh";

function __toString() {

		return "works";

}
}
echo( json_encode( new A() ) );


{"youshouldnotseethis":"uh oh"}

csteipp added a comment.Via ConduitJan 2 2013, 5:33 PM

Patch looks reasonable to me, and looks like it should apply fine to both master and 1.20.

We'll get the cluster patched asap, and this will be in the 1.20.3 release.

With this, and admin could easily block and unblock a user to get their password hash, and attempt to escalate their privileges. And if they escalated to someone with suppress rights, they could cover their tracks. So I think this may drive getting 1.20.3 out a little sooner.

siebrand added a comment.Via ConduitFeb 8 2013, 11:45 AM

Should 1.19 also get this?

csteipp added a comment.Via ConduitFeb 9 2013, 12:21 AM

Yes, 1.19 should also get this. Shooting for a 1.20.3/1.19.4 release next week.

csteipp added a comment.Via ConduitFeb 16 2013, 12:52 AM

Reviewing this for release and realized this throws an exception if the block was an autoblock -- "Fatal error: Call to a member function getName() on a non-object".

Probably just need a $target instanceof User ? conditional.

csteipp added a comment.Via ConduitFeb 16 2013, 12:57 AM

Created attachment 11796
Updated Patch

Attached: wmf10.patch

csteipp added a comment.Via ConduitFeb 16 2013, 1:06 AM

Created attachment 11797
patch for 1.19 branch

Attached: REL_119.patch

csteipp added a comment.Via ConduitFeb 21 2013, 9:21 PM

Can I get someone on this bug to "+2" the attached "Updated Patch", and I'll deploy it to cluster? It's a one line change to includes/api/ApiUnblock.php.

Krenair added a comment.Via ConduitFeb 21 2013, 9:35 PM

(In reply to comment #10)

Created attachment 11796 [details]
Updated Patch

+2: Looks good to me

Attached: wmf10.patch

csteipp added a comment.Via ConduitMar 4 2013, 7:13 PM

Released as part of 1.20.3 / 1.19.4

csteipp added a comment.Via ConduitMar 5 2013, 5:40 PM

RedHat as assigned CVE-2013-1817 to this issue.

csteipp added a project: Security.Via WebMar 26 2015, 8:39 PM

Add Comment