r101761 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r101760‎ | r101761 | r101762 >
Date:00:26, 3 November 2011
Author:preilly
Status:ok (Comments)
Tags:
Comment:
fix for bug 32160 - MobileFrontend search suggestions formatting looks like it is full of XSS/HTML injection vectors
Modified paths:
  • /trunk/extensions/MobileFrontend/javascripts/opensearch.js (modified) (history)

Diff [purge]

Index: trunk/extensions/MobileFrontend/javascripts/opensearch.js
@@ -144,20 +144,30 @@
145145 }
146146 }
147147
 148+function htmlEntities( str ) {
 149+ return String( str ).replace( /&/g, '&amp;' ).replace( /</g, '&lt;' ).replace( />/g, '&gt;' ).replace( /"/g, '&quot;' ).replace( /'/g, '&#39;' );
 150+}
 151+
 152+function escapeJsString( str ) {
 153+ return String( str ).replace( /\\/g, '\\\\' ).replace( /'/g, "\\'" ).replace( /\n/g, '\\n' );
 154+}
 155+
148156 function writeResults( sections ) {
149157 results.style.display = 'block';
150158 if ( !sections || sections.length < 1 ) {
151159 results.innerHTML = "No results";
152 - } else {
 160+ } else {
153161 var html = '<div class="suggestions-results">';
154162 for ( i = 0; i < sections.length; i++ ) {
155163 var section = sections[i];
156164 var rel = i + 1;
157165 section.value = section.value.replace( /^(?:\/\/|[^\/]+)*\//, '/' );
158 - html = html + "<div class=\"suggestions-result\" rel=\"" + rel + "\" title=\"" + section.label + "\"><a class=\"sq-val-update\" href=\"javascript:sqValUpdate('" + section.label + "');\">+</a><a class=\"search-result-item\" href='" + section.value + "'>" + section.label + "</a></div>";
 166+ console.log(htmlEntities(escapeJsString( section.label)));
 167+ html = html + "<div class=\"suggestions-result\" rel=\"" + htmlEntities( rel ) + "\" title=\"" + htmlEntities( section.label ) + "\"><a class=\"sq-val-update\" href=\"javascript:sqValUpdate('" + htmlEntities( escapeJsString( section.label ) ) + "');\">+</a><a class=\"search-result-item\" href='" + htmlEntities( section.value ) + "'>" + htmlEntities( section.label ) + "</a></div>";
159168 if ( i < ( sections.length - 1 ) ) {
160169 html = html + '<hr />';
161170 }
 171+ console.log(html);
162172 }
163173 html = html + '</div>';
164174 results.innerHTML = html;

Follow-up revisions

RevisionCommit summaryAuthorDate
r1017621.18wmf1: MFT r101761preilly00:27, 3 November 2011

Comments

#Comment by Yair rand (talk | contribs)   01:27, 3 November 2011

The console.log needs to be removed. It will break most browsers. And this really shouldn't use innerHTML.

#Comment by Preilly (talk | contribs)   02:24, 3 November 2011

The console.log was there on accident it has been fixed in r101776.

Status & tagging log