1008051 - Malformed HEAD's meta property="og:description" leaking URLs in the top of the listings.

Status: VERIFIED FIXED

(addons.mozilla.org Graveyard :: Public Pages, defect, P1)

Description

[Diego Casorran [:diegocr]]

The <meta property="og:description" .../> is wrongly generated for addon listings containing URLs in their summary, which causes these urls to be shown at the top of the pages.

For example: https://addons.mozilla.org/en-US/firefox/addon/integrated-gmail/

Comment 1

[Wil Clouser [:clouserw]]

Thanks for the bug.  I'm flagging security sensitive since this is a pretty big issue.  /CC Yohan & Mathieu - would you look at this tomorrow?  We'll push a fix offcycle once it lands.  Thanks.

Comment 2

[Mathieu Agopian [:magopian]]

This was introduced by fixing https://bugzilla.mozilla.org/show_bug.cgi?id=998745 with https://github.com/mozilla/olympia/commit/d4ccc97030b143c719d81c67b2e03c74cf6853ca

Comment 3

[Mathieu Agopian [:magopian]]

This issue _should_ not be of any security concern: it _should_ only display links that we forge ourselves, and escape any other markup. According to a few tests, there's no XSS risks on this particular bug.

Comment 4

[Mathieu Agopian [:magopian]]

PR: https://github.com/mozilla/olympia-security/pull/7

Comment 5

[Mathieu Agopian [:magopian]]

fixed in https://github.com/mozilla/olympia/commit/266144ca0d63a841f68482081540c09b8fe36a79

Comment 6

[Diego Casorran [:diegocr]]

While that commit should fix the main problem reported here, i think that needs more sanitization. For example summaries containing double quotes will also cause a malformed meta.

Comment 7

[Mathieu Agopian [:magopian]]

No worries about that, the output is automatically escaped and single and double quotes are thus escaped.

This is because the "|striptags" helper changes the input back to unsafe content (which is then automatically escaped).

Comment 8

[Diego Casorran [:diegocr]]

Great then :)

Thanks.

Comment 9

[Wil Clouser [:clouserw]]

I'll remove the security flag then - I thought this was injection, I didn't realize we were doing the linking ourselves.  Thanks  for filing/fixing.

Comment 11

[Iulian Timis]

Please add STR here or mark it with [qa-] if no QA is needed.

Comment 12

[Mathieu Agopian [:magopian]]

STR:

1/ change the summary of any addon to include links
2/ save the modification, and display the addon page: if it shows a piece of a URL at the very top of the page, then the issue is still present.

Also, if you check the page source for the "<meta property="og:description ...>" tag, it should be well formed.

Comment 13

[Iulian Timis]

Verified as fixed in https://addons-dev.allizom.org/ on FF29 (Win 7).
Attaching postfix screenshot.
Closing bug.

Comment 14

[Iulian Timis]

Attachment: postfix_summary_link.png