(10:00:49 PM) racke: ding dong, anyone around for Interchange meeting (10:01:34 PM) hexfusion: hello (10:01:53 PM) ***daniel_b whistles (10:03:02 PM) racke: ok, first for startes I want to announce that I visit Perl Oasis conference in Orlando as speaker (10:03:44 PM) racke: http://perloasis.org/opw2010/talk/2486 (10:03:55 PM) endpoint_david: cool! (10:04:26 PM) racke: hopefully Vend::Wiki is really in production till then (10:04:49 PM) hexfusion: nice time to be in Orlando (10:05:01 PM) matjones: thats my birthday :) (10:05:15 PM) racke: Mr. Oasis :-) (10:05:35 PM) racke: yeah, I'll stay 10 more days after that in Florida (10:05:40 PM) matjones: nice (10:06:14 PM) racke: endpoint_david: do we try Interchange release in 90 minutes :-) (10:06:39 PM) endpoint_david: or your money back! (10:07:08 PM) racke: ha! (10:07:10 PM) endpoint_david: yeah, let's see what's on the table right now, and if there are any other final changes we can get in there (10:07:31 PM) racke: what did we decide about the [data session arg] vulnerability? (10:08:26 PM) endpoint_david: I think we were fairly evenly divided, although my partner in that debate (thunder) decided to skip the meeting (10:08:48 PM) endpoint_david: i.e., to pre-munge or not (10:09:56 PM) racke: what do you think snewpy? (10:10:06 PM) racke: or anyone else :-) (10:10:11 PM) snewpy: oops, wasn't watching here (10:10:14 PM) snewpy: reading now (10:10:54 PM) snewpy: what's the debate about [data session arg]? that it should filter entities before returning? (10:11:27 PM) racke: yes whether to fix it in the tag or in any invocations in standard (10:11:35 PM) endpoint_david: looks like jon_jensen just came online on skype (10:11:41 PM) snewpy: the latter, imo (10:12:16 PM) snewpy: it can be passed to [filter] just like any other tag to prevent xss.. so the bug is really in standard, misusing the tag (10:14:33 PM) racke: well, I don't agree (10:15:27 PM) snewpy_ [n=Mark@2001:44b8:745b:aed0:2147:181e:127b:939c] entered the room. (10:15:31 PM) endpoint_david: I think I remember my position; [data session *] could escape just fine, as long as the underlying data store wasn't changed in-place; that was my beef with the proposed approach (10:15:33 PM) racke: the majority of Interchange users even don't upgrade Interchange core, how likely is it to upgrade standard apges (10:16:29 PM) snewpy_: but changing the way data session arg works will break existing code (10:16:31 PM) endpoint_david: so you could change tag_data to automatically escape on the return (10:17:08 PM) endpoint_david: snewpy_: we did do a code review of the existing usages in standard, and there were no real issues in that regard (10:17:13 PM) endpoint_david: my concern was for code in the field (10:17:17 PM) snewpy_: what if, right now, i have a page that does something like.... [filter op=sql][data session arg][/filter] and I go and use that in a query somewhere... it'd break if we mess with how [data] returns (10:17:25 PM) racke: snewpy_: I doubt that much existing code will break with [data session arg] (10:17:27 PM) snewpy_: yes, I know that standard wouldn't be affected (10:17:35 PM) snewpy_: but other peoples' will undoubtedly (10:17:43 PM) racke: how? (10:17:49 PM) snewpy_: my example above (10:18:19 PM) racke: well that would produce '[data session arg]' (10:18:24 PM) snewpy_: any non-display usage of [data session arg] is liable to break (10:18:37 PM) snewpy_: well, insert interpolate=1 where appropriate :P (10:19:21 PM) ***racke sighs (10:19:29 PM) snewpy_: also, what about existing code that properly uses [data session arg] wrapped around a filter? (10:19:37 PM) snewpy_: what will the effect of filtering it twice have on the result (10:19:41 PM) endpoint_david: I believe gert pointed out that for most cases [data] already returns entity-escaped values (10:20:11 PM) racke: nothing because it is quite reasonable that [data session arg] doesn't contain parens and all that XSS fun (10:21:45 PM) endpoint_david: IIRC, we also looked at the safe_data regex (10:22:14 PM) snewpy_: modifying what [data session arg] returns is clearly fixing the problem in the wrong place, so in my mind there'd have to be a pretty compelling reason to do so (10:22:22 PM) snewpy left the room (quit: Read error: 60 (Operation timed out)). (10:22:44 PM) racke: isn't security compelling (10:22:49 PM) snewpy_: I think we should use it to remind people that they must filter all external data for safety (10:22:54 PM) racke: I would think so (10:22:58 PM) racke: muhahaha (10:23:14 PM) snewpy_: security is indeed compelling, but catering to users who don't care enough to upgrade for security releases... they pretty much deserve what they get imo (10:23:17 PM) endpoint_david: racke prepares his RackeBot to take over unpatched IC sites (10:23:25 PM) jon_jensen [n=jon@dsl-216-128-233-19.teton.id.tetontel.com] entered the room. (10:23:50 PM) racke: hello jon_jensen (10:24:12 PM) racke: relying on the users doesn't lead anywhere (10:24:19 PM) jon_jensen: hi (10:24:21 PM) snewpy_: recommending wrapping [data session arg] in a filter would seem to be the right thing here.. we update standard, publish an advisory... that way, people don't need to upgrade for the fix, just address it in their catalogs (10:24:40 PM) ***racke looks for his backward compatibility free zone (10:24:53 PM) snewpy_: this isn't a backwards compat argument in my opinion (10:25:05 PM) snewpy_: this is fixing the wrong problem for the right reason (10:25:21 PM) snewpy_: you know my opinion on most BC stuff racke :) (10:25:32 PM) racke: well it's about a big obstacle when changing about anything (10:25:45 PM) racke: like mainframe SKUs O:-) (10:26:12 PM) snewpy_: but i think changing [data session arg] is not the right answer, regardless of it breaking existing code or not (10:26:22 PM) snewpy_: [data session arg] isn't a display function, it's a data retrieval function (10:26:26 PM) snewpy_: it shouldn't randomly munge data (10:26:53 PM) daniel_b: I just installed the new post-receive hook on elmo and it's not working (push went ok with no errors, but no e-mail was sent.) I'd like to check syslog and the MX log files but they're root only. (10:26:56 PM) racke: I think it's reasonable to do the right thing by default in our code, maybe putting a configuration option/pragma in place to use the former behaviour (10:27:03 PM) snewpy_: the problem is it was used, unfiltered, as a display function... change standard, make a release, announce it, remind people to check *all* unfiltered remote data (10:27:25 PM) snewpy_: but munging data inside that tag isn't the "right thing" (10:27:35 PM) snewpy_: if [data session arg] was used *only* for html display, i'd be all for it (10:27:55 PM) jon_jensen: anyone want to email me the discussion so far so I can catch up? (10:28:23 PM) snewpy_: i'll pastebin it (10:28:44 PM) racke: we are munging [data] already (10:29:14 PM) snewpy_: http://www.paste.me.uk/1371.html (10:29:19 PM) racke: you can't put ITL in there by default for instance (10:30:00 PM) snewpy_: yes, but that's separate to html, which the server does not interpret at all (10:30:26 PM) snewpy_: ITL is a special case, because that's the server's interpreted language, so there's a clear benefit in not executing arbitrary code on the server side (10:30:53 PM) snewpy_: and no obvious way to filter it if you didn't automatically do so at the back end of that tag, unlike [filter op=entities] (10:31:17 PM) jon_jensen: snewpy_: thanks (10:31:23 PM) snewpy_: jon: de nada (10:32:12 PM) snewpy_: your average interchange store that has been hacked together probably has several other xss vulns, too (10:32:33 PM) racke: that's no reason to keep them (10:32:40 PM) snewpy_: which in my mind makes it more compelling to not just munge data session arg, but to use this as an opportunity to remind people to not parrot back untrusted user input (10:32:59 PM) snewpy_: right (10:33:29 PM) snewpy_: my point exactly... addressing one by hiding the problem that input was unchecked doesn't draw peoples' attention to the real problem (10:33:47 PM) racke: ok, so who is doing the patch? (10:34:50 PM) snewpy_: there's a patch already, right? (10:35:04 PM) snewpy_: http://github.com/jlavin/interchange/commit/1abf12d5332b57d50843198ab9b159778c491297 (10:35:11 PM) racke: incomplete (10:35:23 PM) racke: that fixes just one invocation of it (10:35:41 PM) snewpy_: ah I see your email.. it's misused all around the place (10:37:52 PM) matjones_ [n=chatzill@c-69-181-63-55.hsd1.ca.comcast.net] entered the room. (10:40:17 PM) snewpy_: taking a quick glance at standard.. looks like there's some other suspect cgi parameter usage that isn't [data session arg] too (10:40:57 PM) snewpy_: standard/pages/member/ship_addresses.html use of [cgi destination] is pretty suss (10:41:14 PM) snewpy_: same for bill_addresses.html too (10:41:32 PM) endpoint_david: ok, Vend::Interpolate::tag_data:788 or so (10:41:42 PM) endpoint_david: 786 (10:42:12 PM) endpoint_david: would adding $opt->{filter} ||= 'entities' work for this fix? (10:42:37 PM) RageLink left the room (quit: brown.freenode.net irc.freenode.net). (10:42:37 PM) endpoint_david: i.e., we're restricting to [data session] (10:42:42 PM) endpoint_david: so we'd not be munging user data (10:43:20 PM) endpoint_david: and none of the values in session should be coercible by the user anyway (10:43:22 PM) RageLink [n=ragelink@190.10.50.199] entered the room. (10:43:58 PM) endpoint_david: my concerns with adding the functionality to safe_data is the behavior change in user code in [data] in general, particularly if html was previously being stored in a database field (10:44:08 PM) endpoint_david: it would no longer be interpolated whereas it was before (10:44:18 PM) endpoint_david: this also addresses my concern with munging the data in place (10:44:51 PM) endpoint_david: (I'm presuming that filter_value does not actually change the data in place, so more like [scratch filter=entities keep=1]) (10:45:11 PM) racke: ok but I don't see the usecase where we store HTML in mv_arg (10:45:15 PM) endpoint_david: this strikes a good balance between purity (i.e., don't munge data) and practicality (XSS attack) (10:45:29 PM) endpoint_david: racke: we don't, but if you're using [data table field] (10:45:45 PM) endpoint_david: i.e., we are restricting it to the smallest range of values (10:46:09 PM) racke: even I won't change [data table field] :-) (10:46:18 PM) endpoint_david: BC be damned! (10:46:19 PM) snewpy_: but this ignores the fact there are other non [data session arg] xss vulns in standard (10:46:21 PM) endpoint_david: ;-) (10:46:30 PM) endpoint_david: snewpy_: one down, though (10:46:39 PM) snewpy_: which is why I think it's the wrong place to address it (10:46:45 PM) endpoint_david: and if they're [data session *] this would take care of it (10:46:52 PM) snewpy_: no, these are [cgi] (10:46:57 PM) racke: drop standard altogether O:-) ? (10:47:32 PM) endpoint_david: hmm, so looks like you could screw things up by creating/naming a database 'session' (10:47:34 PM) endpoint_david: :-) (10:48:12 PM) snewpy_: i think it'd be better to write a script that went thru all the pages and switched every single instance of [cgi] and [data session *] to have a filter (10:48:17 PM) snewpy_: that'd be far more bang for buck (10:49:17 PM) racke: i would argue, you could escape them instead if you switch every single instacne (10:49:19 PM) endpoint_david: snewpy_: perhaps, but that certainly won't make this release, nor will it fix any issues for existing catalogs (10:49:38 PM) snewpy_: the idea being that such a script would fix existing catalogs (10:49:40 PM) snewpy_: in a way that this will not (10:49:51 PM) snewpy_: because it'd pick up other dodgy uses of [cgi] and [data session *] (10:50:57 PM) snewpy_: so we'd provide a "quick" way, based on a script, to fix up peoples' sites who are based on standard (10:51:53 PM) racke: but it would also mess up places where [cgi] isn't used for display (10:52:11 PM) snewpy_: haha i sense the reverse argument now ;) (10:52:58 PM) hexfusion: scratches head (10:53:07 PM) matjones left the room (quit: Read error: 110 (Connection timed out)). (10:53:14 PM) racke: no my argument if you do it with a script, you can just change it in core (10:53:34 PM) snewpy_: well no, you give the option of using the script or looking yourself (10:54:04 PM) snewpy_: changing it in core removes that option, to save the small portion of users who are conscientious enough to upgrade, not not enough so as to look at their pages for xss vulns (10:54:35 PM) snewpy_: because we're talking about a very small segment of users who actually do upgrade for fixes but don't care enough to fix what we suggest they fix (10:54:53 PM) snewpy_: i imagine that most people who don't bother to fix things also don't bother to upgrade, and changing something in core doesn't address them either (10:55:35 PM) racke: it's still more likely that they upgrade core in some distant future (10:55:47 PM) snewpy_: a lazy search and replace script is useable by everyone who is lazy, even those who wouldn't or couldn't upgrade IC... a change in core only fixes it when people upgrade (10:56:08 PM) snewpy_: take a look at how many sites have upgraded since the last security vuln (10:56:16 PM) snewpy_: which was far worse than an xss attack (10:56:55 PM) racke: true (10:57:23 PM) racke: one problem is that Interchange isn't really integrated into distributions (10:57:28 PM) snewpy_: so I think we need to work from the presumption that most people will not upgrade (which we should also discuss separately.. how to better keep people informed, how to address whatever is making it difficult for ppl to upgrade, etc) (10:57:49 PM) snewpy_: well also, the fact our current stable release is a brown paper bag release doesn't help matters (10:59:14 PM) racke: the fact is also we are facing too many problems and have only minimal manpower (10:59:57 PM) snewpy_: right (11:01:14 PM) racke: so where should we use our manpower? (11:01:23 PM) snewpy_: looking at the hall of fame, in my quick test, more than half still vulnerable to the search vuln (11:01:52 PM) snewpy_: imo, we should fix standard to be correct (11:01:55 PM) racke: looks like an excellent business opportunity (11:02:11 PM) snewpy_: then "distribute" our problem, to wider manpower... by warning users to fix their catalogs to prevent XSS (11:02:17 PM) racke: offer all these sites an upgrade (11:02:39 PM) snewpy_: and then spend the time we're spending arguing about changing core to plaster over it making some tough decisions about releasing something that is able to be upgraded to by most people :) (11:02:48 PM) snewpy_: racke: indeed (11:03:16 PM) rsiddall [n=chatzill@24-151-5-218.static.nwtn.ct.charter.com] entered the room. (11:03:44 PM) endpoint_david: on further review, I'm not convinced that my proposed patch is the right place any way (11:04:50 PM) racke: ok so we write a script that spots these places in standard, fix them in Git and make a release of both 5.6 and 5.7? (11:07:03 PM) batrams left the room (quit: "Page closed"). (11:08:00 PM) snewpy_: i think that would be a reasonable balance (11:08:52 PM) jon_jensen: +1 (11:09:06 PM) Orlando-: Sounds like an interesting discussion :D (11:09:33 PM) racke: ok any volunteers :-) (11:09:46 PM) Orlando-: What needs doing? :) (11:09:52 PM) snewpy_: don't have commit access ;) (11:10:23 PM) jon_jensen: snewpy_: this can all be done in GitHub! (11:10:29 PM) endpoint_david: snewpy_: I'd expect that the script should recognize and ignore any cgi/data element with a filter attribute, and (if possible) within a [filter] block (11:11:05 PM) jon_jensen: s/ignore/flag for manual review/ (11:11:18 PM) endpoint_david: although there are certainly many potential failures: [filter op=entities interpolate=1]My variable [cgi var] is XSS-safe![/filter] (11:11:34 PM) snewpy_: endpoint_david: yeah, I'd say that'd be reasonable.. or like Jon said, just warn about anything that's not completely obvious (11:11:53 PM) snewpy_: I'm sure someone can craft a magic regexp using lookbehinds to match that kind of stuff? (11:11:56 PM) snewpy_: regexps not my forte ;) (11:13:26 PM) jon_jensen: racke: what other topics are on the agenda? (11:13:37 PM) jon_jensen: we want to be ready, and do, release 5.7.4, right? (11:13:42 PM) racke: yes (11:13:53 PM) jon_jensen: are there any blockers now? (11:14:55 PM) racke: no known regressions compared to 5.7.3, so no (11:15:06 PM) jon_jensen: ok (11:15:24 PM) snewpy_: are there any major things blocking us calling a 5.7 release 5.8 soon? (11:15:53 PM) endpoint_david: I'd say until all UTF8 issues are resolved, personally (11:15:59 PM) racke: indeed (11:16:02 PM) endpoint_david: then maybe jump to 6.0... :-) (11:16:18 PM) jon_jensen: nah ... (11:16:23 PM) snewpy_: save 6.0 for destroying backwards compat ;) (11:16:25 PM) ***snewpy_ ducks (11:16:34 PM) jon_jensen: 6.0 oughta have big new feature we can trumpet (11:16:41 PM) jon_jensen: "UTF-8" was a new feature years ago :) (11:17:02 PM) snewpy_: haha (11:17:13 PM) endpoint_david: "finally works as advertised" isn't a catchy slogan? ;-) (11:17:20 PM) jon_jensen: it's pretty good if it's true :) (11:17:30 PM) carlbailey: not many can say it (11:17:31 PM) snewpy_: what stands in our way utf-8 wise? aside from the spawn of satan^W^W^WSafe (11:17:39 PM) racke: mail headers (11:17:51 PM) snewpy_: is there a patch for mail headers? (11:18:05 PM) racke: anyone great area where someone you could do a patch (11:18:19 PM) snewpy_: because i'm yet to be convinced it's a bug, so I don't have much to contribute on the topic :) (11:18:50 PM) racke: because IC sends emails not compliant with RFC and get Spam points ? (11:19:22 PM) snewpy_: i played around with it some more after we spoke last and i couldn't find a test case that didn't render correctly in any email client i have (11:19:49 PM) snewpy_: but that string that we were using as an example is valid according to the RFC, by my reading at least (11:19:57 PM) racke: that doesn't make it correct (11:20:14 PM) racke: you can write a lot of bad HTML code which still displays correct (11:20:53 PM) endpoint_david: yeah, not going to rehash that from last week; the output was broken (11:22:02 PM) racke: if no one bothers to deliver a patch, I might just apply my mime encode filter (11:24:10 PM) snewpy_: the underlying encoding may be broken, but it's rfc compliant... the rfc doesn't specify the internal coding, just a way to represent arbitrary encodings... that string we were playing with didn't trip up the latest spamassassin's default configuration either... doesn't have any rules that matched (11:25:08 PM) snewpy_: perhaps the bug could get an example of broken vs. correct encoding if a patch isn't forthcoming? (11:25:19 PM) endpoint_david: snewpy_: I gave an example last discussion (11:25:30 PM) snewpy_: i'd be happy to work on it and make a patch, i just don't see the bug (11:25:45 PM) snewpy_: but the example you gave rendered incorrectly here, and then the discussion petered off (11:25:58 PM) GodFather left the room (quit: "Ex-Chat"). (11:26:28 PM) racke: ok (11:26:47 PM) endpoint_david: racke: did you post the logs from last week? (11:27:07 PM) racke: endpoint_david, jon_jensen: do you need anything for 5.7.4 release? (11:27:33 PM) racke: http://ftp.icdevgroup.org/interchange/meeting/irc_meeting_20091201.txt (11:29:01 PM) jon_jensen: racke: not that I can think of (11:29:08 PM) jon_jensen: endpoint_david is going to head it up and I'll help out (11:29:18 PM) endpoint_david: sounds good to me (11:30:04 PM) racke: ok (11:31:08 PM) racke: I'll leave soon for my train, so the exercises for next week are writing this script :-), I'll produce an example of broken encoding within mail headers (11:31:13 PM) jon_jensen: racke: you should wake up in the morning to a freshly-baked release :) (11:31:32 PM) racke: ok and bake the Debian packages (11:34:09 PM) endpoint_david: nothing smells better than a batch of Debian in the morning (11:34:55 PM) jeff_b left the room. (11:36:27 PM) racke: ok good bye, see you next time