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

[fu]

[commit: http://hg.dwscoalition.org/dw-free/rev/9dbed85663a9]

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

Delete the name on embedded iframes. Append a random string to the iframes
we generate; also give them an id to help with caching.

Patch by fu.

Files modified:

• cgi-bin/LJ/CleanHTML.pm
• cgi-bin/LJ/EmbedModule.pm
• t/clean-embed.t

--------------------------------------------------------------------------------
diff -r bae45423daa8 -r 9dbed85663a9 cgi-bin/LJ/CleanHTML.pm
--- a/cgi-bin/LJ/CleanHTML.pm	Tue Feb 15 11:28:32 2011 +0800
+++ b/cgi-bin/LJ/CleanHTML.pm	Tue Feb 15 11:28:32 2011 +0800
@@ -442,6 +442,9 @@ sub clean
                         }
                         next TOKEN;
                     }
+
+                    # remove the name, because it can be targetted by links
+                    delete $attr->{name};
                 }
             }
 
diff -r bae45423daa8 -r 9dbed85663a9 cgi-bin/LJ/EmbedModule.pm
--- a/cgi-bin/LJ/EmbedModule.pm	Tue Feb 15 11:28:32 2011 +0800
+++ b/cgi-bin/LJ/EmbedModule.pm	Tue Feb 15 11:28:32 2011 +0800
@@ -334,12 +334,14 @@ sub module_iframe_tag {
     $height = MAX_HEIGHT if $height > MAX_HEIGHT;
 
     # safari caches state of sub-resources aggressively, so give
-    # each iframe a unique 'name' attribute
-    my $id = qq(name="embed_${journalid}_$moduleid");
+    # each iframe a unique 'name' and 'id' attribute
+    # append a random string to the name so it can't be targetted by links
+    my $id = "embed_${journalid}_$moduleid";
+    my $name = "${id}_" . LJ::make_auth_code( 5 );
 
     my $auth_token = LJ::eurl(LJ::Auth->sessionless_auth_token('embedcontent', moduleid => $moduleid, journalid => $journalid, preview => $preview,));
     my $iframe_tag = qq {<iframe src="http://$LJ::EMBED_MODULE_DOMAIN/?journalid=$journalid&moduleid=$moduleid&preview=$preview&auth_token=$auth_token" } .
-        qq{width="$width" height="$height" allowtransparency="true" frameborder="0" class="lj_embedcontent" $id></iframe>};
+        qq{width="$width" height="$height" allowtransparency="true" frameborder="0" class="lj_embedcontent" id="$id" name="$name"></iframe>};
 
     my $remote = LJ::get_remote();
     return $iframe_tag unless $remote;
diff -r bae45423daa8 -r 9dbed85663a9 t/clean-embed.t
--- a/t/clean-embed.t	Tue Feb 15 11:28:32 2011 +0800
+++ b/t/clean-embed.t	Tue Feb 15 11:28:32 2011 +0800
@@ -1,7 +1,7 @@
 # -*-perl-*-
 use strict;
 
-use Test::More tests => 132;
+use Test::More tests => 142;
 use lib "$ENV{LJHOME}/cgi-bin";
 require 'ljlib.pl';
 
@@ -151,6 +151,11 @@ note( "Testing clean_embed (we provide t
     $clean->();
     is( $orig_post, $clean_post, "<iframe> tag: trusted with malicious parameters" );
 
+    $orig_post = qq{<iframe src="http://www.youtube.com/embed/$id" name="thisname"></iframe>};
+    $clean_post = qq{<iframe src="http://www.youtube.com/embed/$id"></iframe>};
+    $clean->();
+    is( $orig_post, $clean_post, "<iframe> tag: with name parameter" );
+
 
     # not sure if we need to do anything about this
     $orig_post = qq{<iframe src="http://www.youtube.com/embed/$id" width="1" height="1"></iframe>};
@@ -420,8 +425,10 @@ note( "Testing parse_embed (We parse the
         # check embed attributes (assumes we only have the one embedded item)
         # make sure that the only top-level iframes we have are the ones we generated
         if ( $viewed_entry =~ "<iframe" ) {
-            my %attrs = $viewed_entry =~ /(name|class|src)="?([^"]+)"?/g;
-            is( $attrs{name}, "embed_" . $u->userid . "_1", "iframe name: $title" );
+            my $userid = $u->userid;
+            my %attrs = $viewed_entry =~ /(id|name|class|src)="?([^"]+)"?/g;
+            is( $attrs{id}, "embed_${userid}_1", "iframe id: $title" );
+            like( $attrs{name}, qr!embed_${userid}_1_[\w]{5}!, "iframe name: $title" );
             is( $attrs{class}, "lj_embedcontent", "iframe class: $title" );
             like( $attrs{src}, qr!^http://$LJ::EMBED_MODULE_DOMAIN/\?journalid=!, "iframe src: $title" );
         }
--------------------------------------------------------------------------------