Add code patterns that could impact privacy to MediaWiki secure code training.
Open, NormalPublic

Description

In an effort to prevent situations like T109638 again, add caution about particular code patterns that could impact privacy in MediaWiki secure code training.

csteipp created this task.Aug 27 2015, 11:40 PM
csteipp updated the task description. (Show Details)
csteipp raised the priority of this task from to Normal.
csteipp claimed this task.
csteipp added a project: Security-Team.
csteipp added a subscriber: csteipp.
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptAug 27 2015, 11:40 PM

Have we identified why User::newFromId(0) was used? (It sounds kind of snotty when I say it like that, that's not my intention, I just mean - is there some area of the documentation we've identified that suggests using it, or anything. I just wonder because, in all of MediaWiki, that call is used only once outside tests, and only in the installer which is pretty obscure. Even in places where an anon user is needed, the preferred way of doing it is usually new User() )

Have we identified why User::newFromId(0) was used? (It sounds kind of snotty when I say it like that, that's not my intention, I just mean - is there some area of the documentation we've identified that suggests using it, or anything. I just wonder because, in all of MediaWiki, that call is used only once outside tests, and only in the installer which is pretty obscure. Even in places where an anon user is needed, the preferred way of doing it is usually new User() )

I think the issue is we haven't called it out as dangerous in any training / documentation, so it didn't raise security questions when it was used. At least that's my take on it.

It might be worth making the user class a bit more explicit. e.g. Having two new factory methods User::newAnonFromCurrentIP() and User::newGenericAnon() (With the later being equivalent to User::newFromName( '127.0.0.2', false ); ). This would make it very clear what the methods do at a glance. It would also allow separating the use case of, I need a generic user object, so my user-generic cache isn't polluted with user specific details [A rather common case in MediaWiki], and the case of, I need an anon user object for the current requester's IP [An extremely obscure need in MediaWiki].

@Bawolff I think those 2 suggestions are very sensible.
I believe User::newFromName( '127.0.0.1', false ) was essentially the expected behaviour of User::newFromId(0);
Also once introduced some explicit documentation of newFromId saying what will likely happen if you pass 0 id.

Aklapper removed csteipp as the assignee of this task.Dec 9 2017, 1:45 PM

"MediaWiki secure code training" = https://www.mediawiki.org/wiki/Security_for_developers and subpages? Or something else?