fu: Close-up of Fu, bringing a scoop of water to her mouth (fu)fu ([staff profile] fu) wrote in [site community profile] changelog,
@ 2012-01-23 12:04 am UTC
  • Previous Entry
  • Add to Memories
  • Tell someone about this!
  • Next Entry

[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 [personal profile] 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
--------------------------------------------------------------------------------

(3 comments) - (Post a new comment)
(Flat) (Top-level comments only)

pne: A picture of a plush toy, halfway between a duck and a platypus, with a green body and a yellow bill and feet. (Martin), My default userpics just about everywhere...

[personal profile] pne
2012-01-23 10:56 am UTC (link)
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".

Last edited 2012-01-23 10:57 am UTC

(Reply to this)  (Thread) 


mark: Photo of Mark's face, taken in standard office fluorescent. (me), Taken in Photo Booth or something.

[staff profile] mark
2012-01-23 11:20 am UTC (link)
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.

(Reply to this)  (Thread from start)  (Parent) 


denise: Image: Me, facing away from camera, on top of the Castel Sant'Angelo in Rome (me, standing outside a broken phone booth)

[staff profile] denise
2012-01-23 11:29 am UTC (link)
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!)

(Reply to this)  (Thread from start)  (Parent) 


(3 comments) - (Post a new comment)
(Flat) (Top-level comments only)