[dw-free] http://bugs.dwscoalition.org/show_bug.cgi?id=4185

[fu]

[commit: http://hg.dwscoalition.org/dw-free/rev/78a0be4f6baa]

http://bugs.dwscoalition.org/show_bug.cgi?id=4185

Strip the auth whether the page linked to is see_request.bml, or
see_request.

Patch by kareila.

Files modified:

• cgi-bin/LJ/CleanHTML.pm
• cgi-bin/LJ/Support.pm
• cgi-bin/LJ/TextUtil.pm

--------------------------------------------------------------------------------
diff -r 3c75a7d7814b -r 78a0be4f6baa cgi-bin/LJ/CleanHTML.pm
--- a/cgi-bin/LJ/CleanHTML.pm	Mon Jan 23 07:53:50 2012 +0800
+++ b/cgi-bin/LJ/CleanHTML.pm	Mon Jan 23 08:05:33 2012 +0800
@@ -114,8 +114,8 @@
     # anything to it if $$data contains only invalid content
     my $newdata = '';
 
-    # remove the auth portion of any see_request.bml links
-    $$data =~ s/(see_request\.bml\S+?)auth=\w+/$1/ig;
+    # remove the auth portion of any see_request links
+    $$data = LJ::strip_request_auth( $$data );
 
     my $p = HTML::TokeParser->new($data);
 
diff -r 3c75a7d7814b -r 78a0be4f6baa cgi-bin/LJ/Support.pm
--- a/cgi-bin/LJ/Support.pm	Mon Jan 23 07:53:50 2012 +0800
+++ b/cgi-bin/LJ/Support.pm	Mon Jan 23 08:05:33 2012 +0800
@@ -591,8 +591,8 @@
     my $reqsubject = LJ::trim($o->{'subject'});
     my $reqbody = LJ::trim($o->{'body'});
 
-    # remove the auth portion of any see_request.bml links
-    $reqbody =~ s/(see_request\.bml.+?)\&auth=\w+/$1/ig;
+    # remove the auth portion of any see_request links
+    $reqbody = LJ::strip_request_auth( $reqbody );
 
     unless ($reqsubject) {
         push @$errors, LJ::Lang::ml( "error.support.nosummary" );
diff -r 3c75a7d7814b -r 78a0be4f6baa cgi-bin/LJ/TextUtil.pm
--- a/cgi-bin/LJ/TextUtil.pm	Mon Jan 23 07:53:50 2012 +0800
+++ b/cgi-bin/LJ/TextUtil.pm	Mon Jan 23 08:05:33 2012 +0800
@@ -34,6 +34,16 @@
     return $a;
 }
 
+# check argument text for see_request links, and strip any auth args
+
+sub strip_request_auth {
+    my $a = $_[0];
+    return '' unless defined $a;
+
+    $a =~ s/(see_request\S+?)\&auth=\w+/$1/ig;
+    return $a;
+}
+
 # <LJFUNC>
 # name: LJ::get_urls
 # class: text
--------------------------------------------------------------------------------

Comments

[pne]

What about people who fiddle the links themselves and turn them into /see_request?auth=1234&id=56789 ? That is, reverse the order of parameters?

That won't match the "&auth=" that the sanitiser is expecting, nor will it match the new code which expects at least one non-space character between "see_request" and "auth".

[mark]

This protection has never been designed to make it so people can't ever post links with the auth codes -- it's just to prevent the "oops" copy-pastes.

[denise]

yeah, as mark said, this isn't to prevent people sharing links with auth codes -- obviously you can do that wherever (AIM, email, blah blah). it was put in (on lj) because there were cases of people pasting links to their private-category support requests (terms of service, account payments, etc) and not realizing that'd authorize anyone to see the request as them, thus exposing things like payment information or whatever that they'd put into the request. there were enough cases of people having massive "oh shit!" moments that we (again, back on lj, but that was when mark and i were still working there) put in the auth stripper just to prevent those "oh shit!" moments.

(then on DW, we removed the need for .bml on links, and so the regexp needed updating!)