launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/bing-xss into lp:launchpad.

Commit message:
Fix XSS in presentation of Bing search results.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/bing-xss/+merge/342950

This slipped through due to a mismatch in escaping conventions between the Google and Bing APIs.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/bing-xss into lp:launchpad.

=== modified file 'lib/lp/services/sitesearch/__init__.py'
--- lib/lp/services/sitesearch/__init__.py	2018-03-27 17:43:27 +0000
+++ lib/lp/services/sitesearch/__init__.py	2018-04-10 16:34:58 +0000
@@ -39,6 +39,7 @@
     urlfetch,
     )
 from lp.services.webapp import urlparse
+from lp.services.webapp.escaping import structured
 
 
 @implementer(ISearchResult)
@@ -489,6 +490,11 @@
                 # should not be indexed.
                 continue
             summary = summary.replace('<br>', '')
+            # Strings in Bing's search results are unescaped by default.  We
+            # could alternatively fix this by sending textFormat=HTML, but
+            # let's just do our own escaping for now.
+            title = structured('%s', title).escapedtext
+            summary = structured('%s', summary).escapedtext
             page_matches.append(PageMatch(title, url, summary))
 
         return PageMatches(page_matches, start, total)

=== modified file 'lib/lp/services/sitesearch/doc/bing-searchservice.txt'
--- lib/lp/services/sitesearch/doc/bing-searchservice.txt	2018-03-28 21:28:12 +0000
+++ lib/lp/services/sitesearch/doc/bing-searchservice.txt	2018-04-10 16:34:58 +0000
@@ -348,6 +348,22 @@
     >>> len(page_matches)
     0
 
+The 'snippet' is not HTML-escaped; we must do that ourselves.
+
+    >>> json_file_name = path.join(base_path, 'bingsearchservice-xss.json')
+    >>> with open(json_file_name, 'r') as json_file:
+    ...     data = json_file.read()
+    >>> page_matches = bing_search._parse_bing_response(data)
+    >>> len(page_matches)
+    1
+    >>> page_matches[0].title
+    u'Bug #1349491 \u201c[OSSA 2014-027] Persistent &lt;XSS&gt; in the Host
+    Aggrega...\u201d : Bugs ...'
+    >>> page_matches[0].summary
+    u'* Enter some name and an availability zone like this:
+    &lt;svg onload=alert(1)&gt; * Save ... - Persistent XSS in the Host
+    Aggregates interface (CVE-2014-3594) + ...'
+
 
 -------------
 URL rewriting

=== added file 'lib/lp/services/sitesearch/tests/data/bingsearchservice-xss.json'
--- lib/lp/services/sitesearch/tests/data/bingsearchservice-xss.json	1970-01-01 00:00:00 +0000
+++ lib/lp/services/sitesearch/tests/data/bingsearchservice-xss.json	2018-04-10 16:34:58 +0000
@@ -0,0 +1,20 @@
+{
+  "_type": "SearchResponse",
+  "webPages": {
+    "totalEstimatedMatches": -25,
+    "value": [
+      {
+        "id": "https://api.cognitive.microsoft.com/api/v7/#WebPages.0";,
+        "name": "Bug #1349491 “[OSSA 2014-027] Persistent <XSS> in the Host Aggrega...” : Bugs ...",
+        "url": "https://bugs.launchpad.net/horizon/+bug/1349491";,
+        "urlPingSuffix": "DevEx,5154.1",
+        "isFamilyFriendly": true,
+        "displayUrl": "https://bugs.launchpad.net/horizon/+bug/1349491";,
+        "snippet": "* Enter some name and an availability zone like this: <svg onload=alert(1)> * Save ... - Persistent XSS in the Host Aggregates interface (CVE-2014-3594) + ...",
+        "dateLastCrawled": "2018-02-28T04:31:00.0000000Z",
+        "fixedPosition": false,
+        "language": "en"
+      }
+    ]
+  }
+}