| Summary: | Security issue with the use of target="_blank" or window.open() | ||
|---|---|---|---|
| Product: | Koha | Reporter: | Chris Cormack <chris> |
| Component: | Templates | Assignee: | Owen Leonard <oleonard> |
| Status: | CLOSED FIXED | QA Contact: | Testopia <testopia> |
| Severity: | major | ||
| Priority: | P5 - low | CC: | brendan, f.demians, fridolin.somers, gmc, jonathan.druart, julian.maurice, jweaver |
| Version: | unspecified | ||
| Hardware: | All | ||
| OS: | All | ||
| Change sponsored?: | --- | Patch complexity: | Trivial patch |
| Documentation contact: | Documentation submission: | ||
| Text to go in the release notes: | Version(s) released in: | ||
| Circulation function: | |||
| Attachments: |
Bug 16095: Remove target="_blank" when a link refer to an external link
Bug 16095: Remove target="_blank" when a link refer to an external link Bug 16095: Remove target="_blank" when a link refer to an external link |
||
|
Description
Chris Cormack
2016-03-17 21:47:02 UTC
How can we fix it? 1/ Remove all _blank links Or 2/ Less good is <a href="somewhere" rel="noreferrer" target="_blank"> It is only supported by - WebKit-based browsers (Chrome/Safari/Opera...) - added support in 2009 [4] - Firefox completed support in version 37 (March 2015) - Microsoft added support to Edge in Windows 10, but no version of Internet Explorer 3/ <a href="somewhere" rel="noopener" target="_blank"> Only supported by chrome 49+ 4/ <meta name="referrer" content="origin"> This is supported in Firefox+WebKit browsers, not in IE, and partially in Edge. I think we should do 1/ every where we can, 2/ everywhere we can't and add 4/ into the doc-head-open.inc just for good measure Galen, probably one for Evergreen too, in case it hasnt been filed yet git grep _blank **/*.tt **/*.inc|wc -l 45 But some are referring an internal url: git grep _blank **/*.tt **/*.inc|grep '/cgi-bin/koha'|wc -l 15 30 remains. Created attachment 49299 [details] [review] Bug 16095: Remove target="_blank" when a link refer to an external link If you click on a link that opens a new tab/window to another site, that tab has access to the original window through JavaScript. The browsing context is related, even if the domains are totally different. The tab retains access to the original window's object via window.opener, even if you navigate to another page or domain, in the new or original window. Access to the Window object means the new window can use Window.location to open a different URL in the original window, perfect for phishing attacks. Depending on the site's Same-Origin Policy settings, the new window may have access to other parts of the original window's DOM as well. Any 'A HREF' that contains a target of of '_blank' or '_new' or a fixed name is vulnerable. Previous security best practice often suggested creating a random fixed name for an unpredictable namespace - that won't help with this problem! Targets of '_self' and '_parent' are safe. We do not use _new (at first glance) but several _blank. Some are used to refer internal url, we do not need to update or remove them. Others are used to satisfy OPACURLOpenInNewWindow, in these case, we should add the rel="noreferrer" attribute to the a tags. In other cases, we can simply remove them and let the users discover that a mouse has more than one button (we are in 2016, they can do it!) (Sorry, that won't be very readable...)
git grep _blank **/*.js **/*.tt **/*.inc # ignore if we are doing weird stuffs in pl
|grep -v '/cgi-bin/koha' # ignore internal url
|grep -v noreferrer # ignore _blank with noreferrer set
|grep -v koha-tmpl/intranet-tmpl/lib # We won't modify external lib
|grep _blank # just for the color
koha-tmpl/intranet-tmpl/js/browser.js: var newwindow = window.open( $(this).attr('href') + '&searchid=' + me.searchid, '_blank' )
koha-tmpl/intranet-tmpl/js/browser.js: $('a[href*="biblionumber="]').not('a[target="_blank"]').click(function (ev) {
=> Refer to internal links (ok)
koha-tmpl/intranet-tmpl/prog/en/includes/cateditor-ui.inc: $('#title').append(' <a target="_blank" href="' + link.href.replace( '{ID}', parts[1] ) + '">(' + link.title + ')</a>' );
koha-tmpl/intranet-tmpl/prog/en/includes/cateditor-ui.inc: $('#status-tag-info').append( '<a href="' + getFieldHelpURL( field.tag ) + '"
target="_blank" class="show-field-help" title="' + _("Show help for this tag") + '">[?]</a> ' + taginfo.lib );
=> I don't know, please help (?)
koha-tmpl/intranet-tmpl/prog/en/js/cataloging.js: newin=window.open("../authorities/auth_finder.pl?source="+source+"&authtypecode="+authtype+"&index="+tagsubfieldid+"&value_mainstr="+encodeURI(mainmainstring)+"&value_main="+encodeURI(mainstring), "_blank",'width=700,height=550,toolbar=false,scrollbars=yes');
=> Refer to internal links (ok)
koha-tmpl/intranet-tmpl/prog/en/js/xmlControlfield.js: url = ((nodePos.getAttributeNode("url") || nodePos.hasAttribute("url")) && nodePos.getAttribute("url") != "" && nodePos.getElementsByTagName('urltext')[0].textContent != "")?" <a href='" + nodePos.attributes.getNamedItem("url").nodeValue + "' target='_blank'>" + nodePos.getElementsByTagName('urltext')[0].textContent + "</a>":"";
=> Does somebody know and understand this file? (?)
koha-tmpl/intranet-tmpl/prog/en/modules/admin/biblio_framework.tt: <form action="import_export_framework.pl" name="form_defaul" method="get" target="_blank" class="form_export">
koha-tmpl/intranet-tmpl/prog/en/modules/admin/biblio_framework.tt: <form action="import_export_framework.pl" name="form_[% loo.frameworkcode %]" method="get" target="_blank" class="form_export">
koha-tmpl/intranet-tmpl/prog/en/modules/members/boraccount.tt: <a target="_blank" href="printfeercpt.pl?action=print&accountlines_id=[% account.accountlines_id %]&borrowernumber=[% account.borrowernumber %]">Print</a>
koha-tmpl/intranet-tmpl/prog/en/modules/members/boraccount.tt: <a target="_blank" href="printinvoice.pl?action=print&accountlines_id=[% account.accountlines_id %]&borrowernumber=[% account.borrowernumber %]">Print</a>
koha-tmpl/intranet-tmpl/prog/en/modules/tools/manage-marc-import.tt: '<a target="_blank" href="' + record_details_url
koha-tmpl/intranet-tmpl/prog/en/modules/tools/manage-marc-import.tt: '<a target="_blank" href="' + record_details_url
=> Refer to internal links (ok)
(In reply to Jonathan Druart from comment #5) > koha-tmpl/intranet-tmpl/prog/en/includes/cateditor-ui.inc: > $('#title').append(' <a target="_blank" href="' + link.href.replace( '{ID}', > parts[1] ) + '">(' + link.title + ')</a>' ); > koha-tmpl/intranet-tmpl/prog/en/includes/cateditor-ui.inc: > $('#status-tag-info').append( '<a href="' + getFieldHelpURL( field.tag ) + > '" > target="_blank" class="show-field-help" title="' + _("Show help for this > tag") + '">[?]</a> ' + taginfo.lib ); > => I don't know, please help (?) > These are the MARC help links in the statusbar, and should have rel="noreferrer". These are likely not the largest security risks in this list, as the URLs are restricted to the loc.gov and archive.ifla.org domains. One thing that helps with the lack of IE support for noreferrer is that window.opener does not seem to be accessible cross-domain in IE anyway: http://stackoverflow.com/questions/18624351/how-do-i-detect-window-opener-in-ie/18624735#18624735 Created attachment 49339 [details] [review] Bug 16095: Remove target="_blank" when a link refer to an external link If you click on a link that opens a new tab/window to another site, that tab has access to the original window through JavaScript. The browsing context is related, even if the domains are totally different. The tab retains access to the original window's object via window.opener, even if you navigate to another page or domain, in the new or original window. Access to the Window object means the new window can use Window.location to open a different URL in the original window, perfect for phishing attacks. Depending on the site's Same-Origin Policy settings, the new window may have access to other parts of the original window's DOM as well. Any 'A HREF' that contains a target of of '_blank' or '_new' or a fixed name is vulnerable. Previous security best practice often suggested creating a random fixed name for an unpredictable namespace - that won't help with this problem! Targets of '_self' and '_parent' are safe. We do not use _new (at first glance) but several _blank. Some are used to refer internal url, we do not need to update or remove them. Others are used to satisfy OPACURLOpenInNewWindow, in these case, we should add the rel="noreferrer" attribute to the a tags. In other cases, we can simply remove them and let the users discover that a mouse has more than one button (we are in 2016, they can do it!) Signed-off-by: Chris <chrisc@catalyst.net.nz> I have signed the patch off, are we waiting for another patch? Or shall I switch to signed off? It would be great to get this into the upcoming maintenance releases. If possible. (In reply to Chris Cormack from comment #1) > 4/ <meta name="referrer" content="origin"> > > This is supported in Firefox+WebKit browsers, not in IE, and partially in > Edge. We could add a patch for that point, but I don't know how to test it. Let's get this patch in, and then we can do more if we wish. (In reply to Chris Cormack from comment #10) > Let's get this patch in, and then we can do more if we wish. I've asked piano to QA this. Created attachment 49405 [details] [review] Bug 16095: Remove target="_blank" when a link refer to an external link If you click on a link that opens a new tab/window to another site, that tab has access to the original window through JavaScript. The browsing context is related, even if the domains are totally different. The tab retains access to the original window's object via window.opener, even if you navigate to another page or domain, in the new or original window. Access to the Window object means the new window can use Window.location to open a different URL in the original window, perfect for phishing attacks. Depending on the site's Same-Origin Policy settings, the new window may have access to other parts of the original window's DOM as well. Any 'A HREF' that contains a target of of '_blank' or '_new' or a fixed name is vulnerable. Previous security best practice often suggested creating a random fixed name for an unpredictable namespace - that won't help with this problem! Targets of '_self' and '_parent' are safe. We do not use _new (at first glance) but several _blank. Some are used to refer internal url, we do not need to update or remove them. Others are used to satisfy OPACURLOpenInNewWindow, in these case, we should add the rel="noreferrer" attribute to the a tags. In other cases, we can simply remove them and let the users discover that a mouse has more than one button (we are in 2016, they can do it!) Signed-off-by: Chris <chrisc@catalyst.net.nz> Signed-off-by: Jesse Weaver <jweaver@bywatersolutions.com> Pushed to Master - Should be in the May 2016 release. Thanks! Patch pushed to 3.22.x, will be in 3.22.5 Pushed to 3.20.x, will be in 3.20.10 |