The "{img ...}" and "{meme ...}" remarkup rules violate the new Content-Security-Policy
Closed, ResolvedPublic
Actions

Assigned To

Authored By

	epriestley
	Mar 7 2018, 11:11 PM

Description

See T4340. See T13099. See https://discourse.phabricator-community.org/t/meme-remarkup-tag-no-longer-working-after-upgrading-to-2018-w09/1210/3.

Both {img ...} and {meme ...} source images from the main Phabricator server ('self') even if a CDN domain is configured. This violates our strict Content-Security-Policy which permits images only from the CDN domain.

These tasks are related, although probably not everything (and perhaps almost nothing) is going to make it into scope:

T4190, improvements to {img ...}.
T12542 but I don't know how to actually pursue that.
T5258, transform code is sorta garbage.
T9818 / T3689 / T5741 / T3562.

A simple fix is to add 'self' to img-src, but I'd prefer not to loosen the CSP if we can help it, and neither of these rules should require sourcing from the primary domain.

Revisions and Commits

rP Phabricator
	D19203	rP10b3ddf42657 Possibly fix memes in email
	D19201	rPa3d282d33efe Somewhat improve meme transform code so it is merely very bad
	D19200	rPc7408f27977d PhabricatorMemeEngine HA HA HA HA
	D19198	rPa099a0626532 Remove some old image transform code with no callsites
	D19196	rP98cac2cc2994 Always serve "{meme ...}" from the CDN domain, never from the primary domain
	D19194	rPb30535a36f5f When rendering "{image ...}" images, check the cache and just render a direct…
	D19193	rP9d3a722eb1fe When proxying an "{image ...}" image fails, show the user an error message
	D19192	rP01bbd71b9683 Separate the "{img ...}" remarkup rule into separate parse and markup phases

Related Objects

Mentioned In: T13099: Plans: 2018 Week 10 Bonus Content
Mentioned Here: T3562: Design Meme/Macro Picker
T3689: Add 'create macro' to 'create meme' dialog
T4190: Rebuild remarkup image proxying
T4340: Implement Content-Security-Policy and Strict-Transport-Security headers
T5258: Memes use older image transform code without modern error handling
T5741: Transparent meme animated gifs show up with a black background
T9818: Memes with multiline texts have their text placed incorrectly
T12542: Build a macro typeahead
T13099: Plans: 2018 Week 10 Bonus Content

Event Timeline

epriestley triaged this task as Normal priority.Mar 7 2018, 11:11 PM

epriestley created this task.

Herald added subscribers: cburroughs, andypuettmann, eadler, revi. · View Herald TranscriptMar 7 2018, 11:11 PM

epriestley mentioned this in T13099: Plans: 2018 Week 10 Bonus Content.Mar 8 2018, 12:09 AM

andy.xu added a subscriber: andy.xu.Mar 8 2018, 4:33 AM

andy.xu removed a subscriber: andy.xu.

nickhutchinson added a subscriber: nickhutchinson.Mar 8 2018, 8:20 AM

epriestley added a revision: D19192: Separate the "{img ...}" remarkup rule into separate parse and markup phases.Mar 8 2018, 1:03 PM

epriestley added a revision: D19193: When proxying an "{image ...}" image fails, show the user an error message.Mar 8 2018, 1:36 PM

epriestley added a revision: D19194: When rendering "{image ...}" images, check the cache and just render a direct "<img />" tag if possible.Mar 8 2018, 2:51 PM

epriestley added a commit: rP01bbd71b9683: Separate the "{img ...}" remarkup rule into separate parse and markup phases.Mar 8 2018, 3:03 PM

epriestley added a commit: rP9d3a722eb1fe: When proxying an "{image ...}" image fails, show the user an error message.

epriestley added a commit: rPb30535a36f5f: When rendering "{image ...}" images, check the cache and just render a direct….

epriestley added a revision: D19196: Always serve "{meme ...}" from the CDN domain, never from the primary domain.Mar 8 2018, 3:40 PM

epriestley added a commit: rP98cac2cc2994: Always serve "{meme ...}" from the CDN domain, never from the primary domain.Mar 8 2018, 3:47 PM

This is technically fixed now but the meme stuff is real old and rough so I'm going to maybe make some kind of effort to get through more of T5258, etc.

Macro awwyiss: quack i am a duck I go quack ha ha ha