Security for developers/Training

We run periodically a Security for developers training session and Q&A. It is targeted to new code contributors, but more experienced developers are also encouraged to participate in the sessions.

Before the session you must watch the MediaWiki Security presentation. Extra bonus if you also check the Secure Coding slides and video.

Watch this page for future Security related training sessions.

Previous sessions
[17:02:33] 	 security office hours starting! [17:02:54] * MaxSem puts on his trolling hat� [17:03:06] * Emufarmers glares at MaxSem.� [17:03:41] 	 Has everyone looked at TIm's talk (link in the email)?> [17:03:46] 	 Aye. [17:03:49] 	 yep [17:03:51] 	 yessir [17:03:55] 	 checkmark [17:03:57] 	 I attended it when he gave it in Berlin ;) [17:04:02] 	 +1 [17:04:35] 	 csteipp, is there any point joining the hangout? [17:04:54] 	 Krenair: Probably not [17:05:04] 	 Let's focus here on IRC first [17:05:10] 	 So to start: Any questions from the videos? [17:05:20] 	 For reference: https://www.mediawiki.org/wiki/Security_for_developers [17:05:33] 	 htmlspecialchars vs htmlentities? [17:05:37] 	 The MUST WATCH video: http://vimeo.com/album/2013929/video/44856793 [17:05:49] 	 your typing hurts my ears csteipp ;-) [17:06:11] 	 (To review: XSS, CSRF, Register globals, SQLi) [17:06:15] 	 The GOOD TO WATCH video: https://commons.wikimedia.org/wiki/File:MediaWiki_Security.ogv [17:06:17] 	 jdlrobson: sorry about that! [17:06:37] 	 ... and the related slides: https://commons.wikimedia.org/wiki/File:TechDays2012-SecureCoding.pdf [17:06:37] 	 gnorlock, htmlentities is more thorough, but MediaWiki has Html and Xml classes that are meant to make it easier still (and do other things). [17:06:38] 	 gnorlock: Good question, and I'll refer you to the php docs for that [17:06:49] 	 We are assuming all you found the time to watch those materials. Question? [17:06:59] 	 Yep, using htmlspecialchars is typically what we use [17:07:11] 	 https://www.owasp.org/index.php/Special:Version is running MediaWiki 1.18 :v [17:07:29] 	 As Tim mentioned, the best way to avoid XSS is using the Html and Xml classes for builders [17:07:56] 	 (includes/Html.php / Xml.php ) [17:08:24] 	 Emufarmers, so hack those security experts:P [17:08:50] 	 MaxSem: right, I was wondering whether it was some sort of interactive demo so people could try out the attacks detailed on the site. :-) [17:09:14] 	 Of special note however, when reviewing code, make sure that you're careful about Html::rawElement, Xml::tags, and Linker::links [17:09:37] 	 Those do *not* escape the html strings that you pass in [17:10:12] 	 Otherwise, most of the Html, Xml, and Linker functions do escape what you pass in based on the context where it's writing out [17:10:22] 	 One thing that Tim didn't mention [17:10:28] 	 was Dom-Based XSS [17:10:37] 	 Is that a familiar topic to anyone? [17:10:51] 	 Yes, generally [17:11:01] 	 E.g. $.html vs. $.text and $.attr [17:11:12] 	 Cool. Does anyone here work in Javascript, and are not familiar with dom-based xss? [17:11:24] 	 superm401: Yep, exactly [17:11:25] 	 I'm not too familiar with it. [17:11:47] 	 I think I see where you're going with that -- but I've not encountered it formally [17:11:51] 	 I haven't heard it be referred to with that name before, but. [17:11:52] 	 Right, so it's especially important when reviewing or writing javascript [17:12:13] 	 I am also not super familiar with it - I was confused as to whether you should 'never' use them, or you should just never use them with user input? [17:12:21] 	 https://www.mediawiki.org/w/index.php?title=DOM-based_XSS [17:12:34] 	 gnorlock: Never without escaped user input [17:12:36] 	 :) [17:12:46] 	 So yes, there are times when you will have to use them [17:12:53] 	 But be very careful about how you do it [17:13:23] 	 Basically, 50% of the XSS vulnerabilities we've fixed on WMF websites have come from javascript [17:13:33] 	 So it's a problem we need to watch out for [17:13:37] 	 this was a pretty good press on classifying xss: http://videos.2012.appsecusa.org/video/54157395 [17:14:03] 	 Thanks dr0ptp4kt, that was a good presentation [17:14:58] 	 So to avoid these type of xss, creating elements, appending attributes by key-value pairs, and then appending the dom object is a good habit to be in [17:15:06] 	 wow 50%? that's surprisingly high [17:15:11] 	 Why are users still allowed to change each other's javascript or have global (skin/'common') JS? [17:15:24] 	 Krenair, only admins can change others' JS, right? [17:15:34] 	 Right. [17:15:35] 	 ergg Common.js :P [17:15:42] 	 By default, yeah. (trwiki is an exception) [17:15:46] 	 What do you mean global, MediaWiki:Common.js or User:Krenair/common.js? [17:15:46] 	 yep, only admin can, and I've used that right to fix a couple of these xss [17:15:57] 	 That seems like a dangerous exception. Why is it allowed? [17:16:00] 	 Common.js doesn't affect some things, no? [17:16:13] 	 superm401, MediaWiki:*.js and User:Krenair/*.js [17:16:20] 	 A wiki that allows anyone to modify other people's js files would be very insecure [17:16:25] 	 superm401, it's still only approved people [17:16:32] 	 Oh, okay. [17:16:46] 	 Cool. Any other questions on XSS? [17:16:55] 	 Isarra, both the site common and user common always load if non-empty. [17:17:12] 	 Even on preferences? [17:17:13] <Wikinaut>	 I did not understand the XSSI example [17:17:22] <Emufarmers>	 superm401: no, not on the login page and such, as I recall [17:17:23] 	 Isarra, it's just arbitrary JS. It can do anything JS can. [17:17:35] <Krenair>	 (on trwiki admins and technicians ('Teknisyenler') have edituserjs) [17:17:35] 	 Emufarmers, true, didn't know that's what he meant. [17:17:36] 	 Wikinaut: I will get to that :) [17:17:41] 	 She I mean. :) [17:17:43] <Isarra>	 I know gadgets don't load on preferences. [17:18:01] 	 Ok, next up is CSRF [17:18:04] <Isarra>	 Ah, login page would be a sensible exception. [17:18:37] <YuviPanda>	 (and that is also why we do not have 'ajax login', IIRC - where the login is just an ajax popup) [17:18:41] 	 Anyone want to give a basic description of what CSRF is? [17:18:48] <Wikinaut>	 csteipp: general info: many pages on MediaWiki are still referring to "edittoken". Should be replaced with "getEditToken" in my view (I was bashed on that to do that by myself...) [17:19:15] <Wikinaut>	 I mean: the security documentation should be up to date [17:19:29] 	 Wikinaut: yep, getEditToken is the right one... I will make a note to update those [17:19:30] <Wikinaut>	 w.r.t. core / master [17:19:33] <RoanKattouw>	 YuviPanda: There are other reasons for that [17:19:43] <Wikinaut>	 csteipp: thank you [17:20:04] 	 for JS - was this the function that Tim was referring to as being removed? mw.user.tokens.get( 'editToken' ) [17:20:15] <YuviPanda>	 RoanKattouw: oh? IIRC that was the primary one, with the other thing being something https related? (vague recollection only, sorry!) [17:20:29] <RoanKattouw>	 YuviPanda: It's only secure if the entire page is delivered over HTTPS [17:20:35] <YuviPanda>	 ah, right. [17:20:43] <RoanKattouw>	 An AJAX popup from an HTTP page basically can't be secure [17:20:45] <YuviPanda>	 pointless to ajax to https [17:20:49] <YuviPanda>	 from http [17:21:06] 	 But our credentials currently go over HTTP anyway, right (by default)? [17:21:06] 	 gnorlock: I don't think that was the one, there was a javascript file that included something like $edittoken=123; [17:21:08] <YuviPanda>	 since any attacker can simply modify the http page / js. [17:21:15] <Wikinaut>	 qgil: can you pls. also add url where this chat is logged ? [17:21:22] <RoanKattouw>	 YuviPanda: Yeah exactly, they can modify the JS that calls the HTTPS [17:22:01] 	 Logins will soon all be going over HTTPS, that is a basic security assumption that we sadly do not enforce by default right now. [17:22:11] <Krenair>	 Wait [17:22:15] <Krenair>	 This is about WMF-specific security? [17:22:26] <Krenair>	 You can't enforce all logins over HTTPS in MediaWiki itself [17:22:34] 	 Krenair, it's already a flag. [17:22:38] 	 Krenair: WMF sites only :) [17:22:41] 	 Though IIRC, there might be a bug. [17:23:05] 	 that's not entirely true - the mobile version of the WMF sites forces https at login [17:23:17] 	 Ok, Wikinaut, since we're talking about XSSI (since it's often used for CSRF), did you have a specific question about it? [17:23:19] 	 awjr, good to know. [17:23:23] 	 Wikinaut, done and good point. I'll copy paste the log of this session elsewhere [17:23:38] <Emufarmers>	 Krenair: sort of: https://www.mediawiki.org/wiki/Manual:$wgSecureLogin [17:23:56] <Krenair>	 Most MediaWikis probably aren't accessible over HTTPS [17:24:04] 	 Really, though, HTTPS only at login is not enough. It doesn't deal with a Firesheep-style attack. [17:24:15] 	 I know we're looking at HTTPS for logged in users everywhere eventually. [17:24:16] <Wikinaut>	 the example was short. need to look to it again. One moment [17:24:24] <Wikinaut>	 csteipp: ^ [17:24:42] 	 The basic concept being that a hostile website loads in a page from our website, [17:24:44] 	 something like [17:24:48] <Emufarmers>	 Krenair: right. That's why it defaults to false (and probably always will) :p [17:25:05] 	 <script src="https://en.wikipedia.org/some_vuln_page" /> [17:25:29] 	 And the javascript on that hostile webpage can read any valid javascript that comes from enwiki [17:25:40] 	 Because they have the same origin (both scripts are loaded from the same webpage) [17:25:53] <RoanKattouw>	 That's.... not strictly true [17:25:58] <RoanKattouw>	 It can *execute* any JS from enwiki [17:26:07] <YuviPanda>	 superm401: on mobile (IIRC) we redirect to https on login and then keep them there [17:26:09] 	 yes [17:26:11] <RoanKattouw>	 And there are two interesting cases where that means you can extract data [17:26:12] <Krenair>	 so my question still hasn't been properly answered as far as I can tell, why are wiki admins still allowed to modify JS? [17:26:24] 	 csteipp: same-origin is based on the document containing the tag, not the location of the script source [17:26:28] 	 So the counter example is loading something from the api using the format=jsonp [17:26:29] <RoanKattouw>	 One is if enwiki is being cooperative and the JS looks like "foo = { data here };" [17:26:33] <RoanKattouw>	 (that's JSONP) [17:26:37] 	 brion: Exactly [17:26:57] <RoanKattouw>	 The other is if the JS that's being loaded is really JSON, but the top-level object is an array and you're dealing with an old browesr [17:27:06] 	 a hostile website can load JS from wikipedia but that JS will have no permission to access wikipedia [17:27:09] <RoanKattouw>	 In that case the attacker can override the Array constructor and capture the data [17:27:30] 	 Side note, that's why top level should never be an array. [17:27:34] <RoanKattouw>	 Yup [17:27:39] 	 exactly [17:27:39] <Emufarmers>	 Krenair: it's fairly useful. Do you think it should be broken out into a separate right or something? [17:27:43] <Isarra>	 Krenair: Same reason we allow volunteer developers, I'd guess - the risks are acceptable and they can only do so much with js. [17:27:50] 	 I know in general sites can read their JS (IIRC textContent). [17:27:55] 	 Is that not true cross-domain? [17:27:57] <RoanKattouw>	 superm401: Not cross-domain [17:28:03] 	 So, this was mentioned as part of csrf, becasue it's common to use an xssi to steal a csrf-protection token [17:28:17] <Wikinaut>	 csteipp: do I understand XSSI example correctly: the first loads a javascript with functions from wikipedia, and the second is                         simply then using properties and especially hijacking the edittoken ? [17:28:43] <Krenair>	 Emufarmers, because, for example on WMF sites, doesn't that open up the possibility for admins to abuse JS to run checkuser? [17:28:55] <Krenair>	 Or access suppressed info? [17:28:55] 	 Krenair: yes but they'd be caught [17:29:13] <YuviPanda>	 I guess enough people watch common.js [17:29:15] 	 Wikinaut: Yeah, typically, in the script would execute set a variable or have a function that contained the edit token. [17:29:16] <Isarra>	 Sure, they can do that. And they'd be beaten half to death for it and it'd be fairly quickly fixed. [17:29:26] 	 Krenair, JS is not enough to run check-user unless you steal cookies or tokens from other users. [17:29:29] 	 Which is pretty obvious. [17:29:35] 	 And then scripts on the hostile webpage can call those function or access those variables. [17:29:35] <Isarra>	 Indeed. [17:29:45] 	 Admin is a trusted position and people are watching these pages (MediaWiki:Common.js). [17:29:47] <Wikinaut>	 But isn't that quite normla ? [17:29:51] 	 It's pretty hard to hide token-stealing code. [17:29:53] <Krenair>	 I thought JS can access cookies and tokens? [17:30:20] <Isarra>	 Similarly if the admin doing that were a concern, same would be general checkuser misuse - and it is, but there's a reason there is such a harsh vetting process. [17:30:24] * jdlrobson wishes Common.js had some form of code review step� [17:30:29] 	 Krenair: It can-- so yes, an admin could escalate their privilates [17:30:41] 	 jdlrobson, could use flaggedrevs in principle. [17:30:57] 	 Don't know if modifications would be needed. [17:30:59] <Krenair>	 The whole feature is just insecure by design really [17:31:02] <YuviPanda>	 jdlrobson: flaggedrevs would be code review for common.js, but I don't see that happening. [17:31:17] 	 Krenair, it's a security trade-off. It's enabled a lot of useful stuff. [17:31:19] <Wikinaut>	 So one of the lessons I have learned is that "view"ed pages should never bear edittokens in it or other sec information [17:31:40] <Wikinaut>	 csteipp: ^ is this correct ? [17:31:46] 	 Wikinaut: Yes, exactly [17:32:03] <Isarra>	 Krenair: Users are insecure by design. [17:32:06] 	 flagged revs would be something in the right direction.. the current situation scares me :) [17:32:11] <Isarra>	 People are a truly problematic factor. [17:32:37] 	 Are there other people watching with a first question? [17:32:39] 	 Another way to get a sort of pseudo-checkuser is to embed an external resource in the .js or .css, and then somehow correlate the server                        logs for that resource with edits or the like. [17:32:58] 	 (just making sure that others have a chance to ask / speak) [17:33:33] <Krenair>	 Oh, actually [17:33:58] 	 (if not just continue)  :) [17:34:04] <Krenair>	 ... no, I shouldn't discuss this in a public channel [17:34:19] 	 there was a laugh in the room here [17:34:26] 	 Ok, moving on to register globals [17:34:30] 	 does action=view not contain any of the js? [17:34:38] 	 We actually had one fixed in our last release [17:34:39] <Isarra>	 Has anyone written up a list on meta of ways to abuse javascript? [17:34:45] 	 And then there used to be a site (might still exist, I haven't looked) that watched recentchanges for IP edits to talk pages followed by a                        logged-in editor "claiming" the comment, to try to log the IP before someone came around and revdeled or oversighted it [17:34:49] 	 seems like every page contains at least some js that says {mw.user.tokens.set({"editToken":"42d7629......"}) [17:35:10] 	 gnorlock: It does, [17:35:20] <RoanKattouw>	 Yes [17:35:24] 	 but the rest of the page around it (should!) be a javascript error [17:35:42] 	 So when the browser executes the included source, it will stop as a result of the parse error [17:35:45] <Emufarmers>	 Isarra: https://www.mediawiki.org/wiki/User:MZMcBride/Attacks has a little bit of that [17:35:52] <RoanKattouw>	 Yeah, <!doctype is where it should crash already [17:36:05] <Isarra>	 Emufarmers: We should update that. [17:36:29] 	 Isarra / Emufarmers, that would be great! [17:36:31] <Wikinaut>	 gnorlock: I tested that now. It does not when running core (master) version [17:36:43] 	 So register globals [17:37:00] 	 Since they are almost deprecated entirely, [17:37:06] 	 I think we often forget to check for them [17:37:07] 	 does register_globals still exist? [17:37:11] <Wikinaut>	 only when I actually clicked on &action=edit and see the input, then the page had my edittoken [17:37:20] 	 Sadly, in php 5.2 and 5.3, they do [17:37:32] 	 brion- Removed in php 5.4 [17:37:36] 	 damn. must been the ill-fated php 6 they were killing it in [17:37:40] 	 oh hey, yay [17:38:00] 	 So yeah, until core only supports 5.4+, we need to make sure to check for them [17:38:00] <YuviPanda>	 core is compatible with 5.2 or 5.3? [17:38:07] <RoanKattouw>	 5.3 now I think [17:38:08] 	 core is 5.3 currently [17:38:21] <Wikinaut>	 gnorlock: I was wrong. You are right [17:38:25] 	 Alright [17:38:32] 	 5.3.2, specifically, if I recall correctly [17:38:34] 	 SQL Injection [17:39:20] 	 Did the SQL Injection part of Tim's talk make sense? [17:39:30] 	 Anyone want to recap it in their own words? [17:40:11] 	 bueller...? [17:40:14] 	 Wikinaut: Yeah, thought it was a bit weird -- but then again I don't fully understand the attack vector on it [17:40:35] 	 Use MW Wrappers, and understand that you can still mess up while using them? [17:40:45] <Emufarmers>	 csteipp: he talked about how even MediaWiki's wrappers could be vulnerable, but I wasn't clear on when that would be the case (just [17:40:53] 	 gnorlock: That works for me :) [17:40:55] 	 https://xkcd.com/327/ [17:40:58] <Wikinaut>	 csteipp: gnorlock: can we talk about this "no edittoken on "view" pages" [17:40:59] 	 Yeah, so use the wrappers [17:41:24] 	 i think when you don't use the array in the WHERE statements, those don't get escaped [17:41:27] 	 also in ->query [17:41:37] 	 Wikinaut: yeah, let's talk more at the end... I'm not quite good enough to do both at the same time :) [17:41:43] <Wikinaut>	 ok [17:41:59] 	 So the issue is that the wrappers to allow you to write in your own sql [17:42:07] <RoanKattouw>	 gnorlock: So array( 'foo' => 'bar' ) is escaped, but array ( 'foo=bar' ) isn't, nor is 'foo=bra' [17:42:09] <RoanKattouw>	 *bar [17:42:13] 	 array( 'user_id' => $user_id ) vs array ( "user_id = $user_id" ) [17:42:14] 	 right [17:42:17] 	 So including user input into those (like for > and < ) can lead to sqli [17:42:24] 	 gnorlock: exactly [17:42:40] <RoanKattouw>	 Yeah, it's sad we don't have wrappers for [17:42:43] 	 And we actually just fixed exactly that one in an extension not too long ago [17:43:01] 	 RoanKattouw has volunteered to write those ;) [17:43:09] <RoanKattouw>	 haha [17:43:18] 	 So yeah, some gotchas [17:43:31] <RoanKattouw>	 (I think I did actually add wrappers for table aliasing a couple years back) [17:43:32] 	 We had an extension that was taking ">" or "<" as user input? Ick. [17:43:37] 	 whenever you concat user data into the sql stmt, use addQuotes [17:43:50] 	 Which will quote, and escape, the data [17:44:01] 	 So run that for each string you're inserting [17:44:34] 	 Also, table names and the keys in the where clause are pretty much raw sql [17:44:41] 	 So don't allow user data into those [17:44:47] <Wikinaut>	 ;-) [17:45:23] 	 SQL Injection is pretty bad, so do always be careful with it [17:45:33] 	 Any other questions on sqli? [17:45:41] <Wikinaut>	 no [17:45:58] 	 Cool. So one more topic I wanted to bring up [17:46:03] 	 That Tim didn't mention [17:46:12] 	 Is the name of the php "mysqli" extension unfortunate? [17:46:23] 	 When you look at the vulnerabilities that we've had, the biggest group by far is XSS [17:46:43] 	 Heh, "MySQL Improved" apparently. [17:46:47] 	 The second largest group is issues where we have been leaking confidential informaiton [17:46:51] 	 anomie: haha [17:47:25] 	 So as you are desinging core and extensions, it's realy important you consider what is private (or could be private) data [17:47:40] 	 Obviously, IP and User Agent are covered by our privacy policy [17:47:59] 	 But it's also important to remember that revisions, images, and username can be hidden or suppressed [17:48:04] 	 So please, please, please [17:48:18] <Wikinaut>	 add: real name [17:48:22] 	 Check if the data base been deleted / supressed before showing it [17:48:27] 	 Wikinaut: Thanks, yep [17:48:55] <YuviPanda>	 just curious - do salted and hashed IPs count? [17:49:00] <Wikinaut>	 I had bad issues due to a recent change with the $wgAllowRealName stuff [17:49:10] 	 YuviPanda: it depends [17:49:15] <Wikinaut>	 which revealed real names in the "contributions" [17:49:23] <Wikinaut>	 should not have happened! [17:49:36] 	 If there are many of them, then the salted/hashed IP can be used to correlate logs and find clickpaths [17:49:39] 	 That would be bad [17:49:52] 	 If it's a single item, then no, probably not [17:50:29] <YuviPanda>	 okay, so the question to ask is 'can this be used in *any* way to tie someone back to this unique idenfitifer?' [17:50:32] <YuviPanda>	 and if the answer is yes, it is private data [17:50:45] 	 YuviPanda: Exactly [17:51:10] <Wikinaut>	 (I would like to ask a question, on topic: I do not understand what you are talking about) [17:51:19] 	 Wikinaut: Yikes, I hadn't realized the functionality changed... That is something to not [17:51:22] 	 *note [17:51:29] <Wikinaut>	 A case, where an extewnsion shows some databases ? [17:51:31] 	 Wikinaut: Go ahead [17:51:49] <Wikinaut>	 csteipp: you talk above about IP addresses and so on [17:52:22] 	 Wikinaut: I'm not sure I understand.. you mean when we show database data, including IP addresses? [17:52:32] 	 Or database name? [17:52:32] <Wikinaut>	 I mean, what scenario is it you are talking about ? [17:52:41] 	 Ah [17:53:11] 	 So the case I meant is sometimes extensions developers will show potentially private data [17:53:20] <Wikinaut>	 ok [17:53:24] 	 Sometimes IP addresses (from the cu table, or recent changes) [17:53:27] <Wikinaut>	 You mean, from the database [17:53:29] <Wikinaut>	 ok [17:53:36] <Wikinaut>	 I understand [17:53:36] 	 Yep, from the DB [17:53:55] <Wikinaut>	 Like I do in E:OpenID, but these are only shown for the logged-in user [17:53:57] 	 Refelecting it from the current user-- not a privacy issue [17:54:01] <Wikinaut>	 in their preferences [17:54:05] 	 Correct [17:54:45] <Wikinaut>	 except the OPenID identifier itself, which is public per se, but - new - has (usually) no username in it [17:55:05] <Wikinaut>	 (off-topic) [17:55:25] 	 Great. So with 5 mins left, any other questions? [17:56:00] <Wikinaut>	 csteipp: just some words about the visible edittoken on normal viewed pages [17:56:06] 	 If not, Wikinaut, we can talk about those yep. [17:56:38] 	 Wikinaut: So the general rule of not showing something like an edit token on the page is good [17:56:54] <Wikinaut>	 yes [17:57:17] <Wikinaut>	 I can tell you what I obeserved. Shall I ? [17:57:18] 	 Action=view will *typically* not be able to execute as javascript [17:57:26] 	 Sure [17:57:49] <Wikinaut>	 (as a logged in user) I visited my Main_page: no edittoken yet [17:57:59] 	 csteipp: can you just go over real quick the action=view security scenario? [17:57:59] 	 Correct [17:58:06] <Wikinaut>	 then I clicked action=edit [17:58:08] 	 What do you mean exactly? [17:58:26] 	 Obviously action=view runs JS. [17:58:29] * qgil reminds that we will be kicked out of this (physical) office in 1 minute� [17:58:46] 	 we can probably still hang our the IRC office [17:59:00] 	 Right, so action=edit puts the token on the page. [17:59:04] <Wikinaut>	 yes [17:59:06] <RoanKattouw>	 superm401: The action=view HTML does not parse as valid JS [17:59:06] 	 Also, your feedback about this sessions and whether / when we should run a next one is welcome. [17:59:10] 	 We do some extra work to protect the edit page [17:59:16] <Wikinaut>	 and then "view" has still that token! [17:59:18] <RoanKattouw>	 So you can't trick the browser into revealing the token by loading in action=view as JS [17:59:44] <Wikinaut>	 RoanKattouw: I am not sure [17:59:46] 	 leavng the room, give us 1 min [18:00:23] 	 Wikinaut: really? thhat shouldnt happen.... i dont think... [18:00:34] <Wikinaut>	 I just tested that. [18:00:51] 	 was the attack basically putting in a <script src='mediawiki/someJsFileWithEditToken.js'/> on a page, and then reading it for the edit token of whoever hit the page? [18:00:54] <Wikinaut>	 I always do "ctrl F5", by the way [18:01:15] 	 Wikinaut: I also have it on all of mine [18:01:15] <Wikinaut>	 (why is that chat time-limited ? I do not understand) [18:01:32] <RoanKattouw>	 Wikinaut: The chat isn't, the room we were in was [18:01:32] <Wikinaut>	 gnorlock: not at the first access [18:01:32] <RoanKattouw>	 So we had to walk back to our desks [18:01:40] <RoanKattouw>	 csteipp: All tokens are loaded on all page views, even action=view [18:01:49] <Wikinaut>	 Roan: You have only that one room ? [18:01:54] <RoanKattouw>	 JS might wish to initiate edits via the API (e.g. Twinkle) [18:02:15] <RoanKattouw>	 Wikinaut: No but someone else had booked it after us so they kicked us out [18:02:17] <Wikinaut>	 So, Dear Sirs, Houston, we have a problem ? [18:02:27] <RoanKattouw>	 I don't think tokens on action=view is a problem [18:02:30] <Wikinaut>	 ^was related to the token [18:02:35] <RoanKattouw>	 If csteipp disagrees I'll defer to him obviously [18:02:43] 	 Is the editToken checked on searches? [18:02:46] 	 csteip is reconnecting... [18:02:50] <Wikinaut>	 RoanKattouw: above you said " The action=view HTML does not parse as valid JS" [18:03:04] <Wikinaut>	 can you pls. explain tthis to me (sorry) [18:03:11] <Wikinaut>	 what JS [18:03:26] <Wikinaut>	 and why "does not parse" [18:03:30] <RoanKattouw>	 Wikinaut: So if you did <script src="http://en.wikipedia.org/wik/Foobar"> [18:03:43] <RoanKattouw>	 The browser would load an action=view page and attempt to interpret it as JS [18:03:50] <RoanKattouw>	 Obviously this is ridiculous and should fail [18:04:02] <RoanKattouw>	 But on the off chance it doesn't, the attacker may be able to grab tokens [18:04:17] <RoanKattouw>	 In MW this should always fail because of <!doctype html> at the start [18:04:40] 	 RoanKattouw, technically, I think HTML5 is optional. [18:04:42] 	 Sorry about that... [18:04:46] 	 But I think it should fail either way. [18:04:54] <RoanKattouw>	 superm401: Technically it is, but this is a good reason to include it [18:05:03] <RoanKattouw>	 csteipp1: So we do actually load tokens on all pages, even action=view [18:05:07] 	 superm401- Then it'll choke on the HTML4 doctype instead [18:05:18] 	 anomie, right, that's what I thought, or the head tag or something. [18:05:21] <RoanKattouw>	 For the benefit of things like Twinkle that initiate AJAX API edits from the view page [18:05:29] 	 RoanKattouw: and a JSONP request to that page would fail because it wouldn't return JSON? [18:05:31] 	 RoanKattouw: So I would probably prefer that didn't happen [18:05:43] 	 I don't think there's a ready way to exploit it [18:06:10] 	 It wouldn't be the end of the world to remove it. [18:06:13] 	 There is https://www.mediawiki.org/wiki/RL/DM#mediawiki.api.edit [18:06:21] 	 which has postWithEditToken that fetches a token if needed. [18:06:26] 	 But I don't see a vector either. [18:06:26] 	 But if someone was able to construct a page + skin that made it valid javascript, that would be bad [18:06:27] <Susan>	 Hello. [18:06:41] 	 Hi Susan [18:07:06] <RoanKattouw>	 superm401: We've documented that mw.user.tokens.get('editToken') will work everywhere so we shouldn't remove it unless there's a                         good reason to [18:07:06] 	 Users can't make their own skins, thankfully (just extend existing ones). And I still think it would be impossible to make a skin that parsed as JS (given HTML skin does not control). [18:07:08] 	 Ok, we are over our office hour, so if anyone else is havnig a session, just yell and we'll get off! [18:07:14] 	 RoanKattouw, completely agreed. [18:07:24] <RoanKattouw>	 gnorlock: Pretty much, yes. A JSONP request is really also just a JS execution [18:07:24] <Susan>	 I watched the security video from Tim the other day. [18:07:30] <Susan>	 There's so much classic Tim in it. [18:07:56] <Emufarmers>	 I had not heard Tim speak before [18:08:00] 	 Hey Susan, we actually were just talking about it for the last hour... but any brief questions on it? [18:08:19] <Susan>	 No, I just enjoyed watching it. [18:08:24] 	 Theres no error exception that you could somehow trigger, and then parse the 'invalid' returned results of the JSONP request (and hence                         search for the edittoken) [18:08:25] 	 ? [18:08:55] 	 gnorlock, JSONP is a normal script. Roan mentioned earlier you can't access text of a cross-domain script. [18:09:14] 	 And action=view does not support JSONP anyway. Only the API does (and not all end-points). [18:09:24] 	 Good point on that-- jsonp always runs as the anonymous user, so edit tokens can't be leaked [18:09:29] 	 ahh ok [18:09:33] <RoanKattouw>	 gnorlock: There is an error callback but it doesn't give you the text, no [18:09:57] <Wikinaut>	 Does anyone know of a good (and "secure") example, how to convert a classic action SpecialPage/action to a safe AJAX way? [18:10:04] <RoanKattouw>	 So you'll know whether it parsed or not, but you won't know what was in it precisely unless both the script parsed and runs in a                         way that tells you its contents (like JSONP does) [18:10:39] 	 Wikinaut: Extending the api is probably the best way [18:11:08] 	 That allows you to set if it requires an edit token [18:11:14] 	 csteipp1: Is wgAjaxExportList considered extending the API? [18:11:28] 	 or is that not used anymore? [18:11:34] * RoanKattouw cringes� [18:11:52] <RoanKattouw>	 AjaxExportList is an ancient framework that isn't the API, you should use the API instead [18:11:53] <Wikinaut>	 csteipp1: do we have a good example already ? [18:11:59] <Wikinaut>	 http://openid-wiki.instance-proxy.wmflabs.org/index.php?title=Special:OpenIDConvert/Delete&url=https%3A%2F%2Flabsconsole.wikimedia.org%2Fwiki%2FUser%3AZ [18:12:00] 	 gnorlock, API is includes/api. [18:12:17] <Wikinaut>	 ^^ above was the action which I want to ajaxify [18:12:34] 	 gnorlock: CentralAuth does a pretty good job [18:12:34] 	 Wikinaut, you can look at https://gerrit.wikimedia.org/r/#/c/44376/18/ApiFancyCaptchaReload.php for a simple API Chris reviewed. [18:12:34] <Wikinaut>	 in a afe way, of course [18:12:38] <Wikinaut>	 thank you [18:12:45] <Wikinaut>	 superm401: ^ [18:12:45] 	 It's for getting a new CAPTCHA instead of reloading the whole form. [18:13:00] 	 Not merged, but I don't think there are any blocking security issues. [18:13:14] <Wikinaut>	 will try to understand it [18:13:22] 	 Wikinaut: I think that should be possible. It would turn into something like api.php?action=delete&url=... [18:13:24] <Wikinaut>	 must use tokens, correct? [18:13:33] <Susan>	 Oh, you need JavaScript snippets? [18:13:37] <Susan>	 With tokens? [18:13:37] 	 Wikinaut, only if you're changing something. [18:13:46] <Susan>	 https://www.mediawiki.org/wiki/Snippets [18:13:46] 	 E.g. an edit, modifying the watchlist, etc. [18:13:53] 	 And use POST for anything that does such a change. [18:13:55] <Wikinaut>	 superm401: the action is only for the logged-in user [18:13:57] <Susan>	 https://www.mediawiki.org/wiki/Snippets/Replace_a_page%27s_contents_with_something_else [18:14:00] <Susan>	 Etc. [18:14:19] <Wikinaut>	 perhaps can someone bake anÂ´super example ? [18:14:30] <Wikinaut>	 also to be put into the sec manuals [18:14:31] 	 Wikinaut, right, but the key question is what the API does. Does it change something? [18:14:45] <Wikinaut>	 do you mean my action ? [18:14:51] 	 Wikinaut, correct. [18:15:00] <Wikinaut>	 it deletes an entry in the db [18:15:09] <Wikinaut>	 then should return to preference tab OpenID [18:15:15] <Wikinaut>	 of that user [18:15:23] <Wikinaut>	 sorry [18:15:27] <Wikinaut>	 correction [18:15:33] 	 Wikinaut, yeah, so I think you will want a token and require POST. [18:15:38] <Wikinaut>	 yes [18:15:39] 	 Yeah, probably want to use post, since you're making an update [18:15:42] <Wikinaut>	 post [18:15:59] <Wikinaut>	 and partial update of the screen [18:16:01] 	 Wikinaut - do you maintain the OpenID extension? [18:16:04] <Wikinaut>	 yes [18:16:09] 	 Note POST does not prevent CSRF, just keeps private data out of the weblogs [18:16:20] 	 And block caching. [18:16:25] 	 Generally. [18:17:25] <Wikinaut>	 As far as I understand, my action would require a two-step request-response-request ajax [18:17:43] 	 Wikinaut: most likely [18:18:06] 	 I think you could potentially preload the token on the form. [18:18:24] <Wikinaut>	 user clicks "delete my Openid 3" xmlhttp -> gets token (via xmlhttp) ->send the final requests [18:18:46] 	 Wikinaut: That is probably the best route to go [18:18:49] <Wikinaut>	 ok [18:19:06] <Emufarmers>	 Thank you for the session, csteipp1, et al. [18:19:15] 	 Emufarmers: You're welcome! [18:19:28] 	 csteipp1, wpEditToken is pre-loaded. Is there a reason not to do that for a prefs screen? [18:19:33] * Emufarmers flees� [18:19:35] 	 Yes, thank you very much. [18:19:36] 	 All, feel free to ping me on irc if you come up with any questions later [18:19:47] <Wikinaut>	 Ah, I am also mainting the AJAXPoll extension, which I like. [18:19:58] <Wikinaut>	 perhaps I can copy and paste from there, what I need [18:20:16] <Wikinaut>	 it uses tokens, I remember [18:20:17] 	 superm401: I'd have to look into it a little more [18:20:36] <Wikinaut>	 I anyone here, who has used that once ? [18:20:37] 	 csteipp1, fair enough. Thanks again. [18:20:46] 	 Bye [18:20:46] <Wikinaut>	 csteipp1: thank you [18:21:33] 	 Thank you everybody! [18:22:20] <Wikinaut>	 if someone has further "pages", please add to the Manual collection: https://www.mediawiki.org/wiki/Manual:MediaWiki_Security_Guide [18:22:34] <Wikinaut>	 which make it nice for download as ebook, or PDF [18:22:41] <Wikinaut>	 bye [18:22:47] 	 thanks Wikinaut [18:23:05] 	 Wikinaut: I sent you a message about OpenID [18:23:08] <Wikinaut>	 qgil: http://bots.wmflabs.org/~wm-bot/logs/%23wikimedia-office/?C=M;O=D