1767877 - csp "inline-style" violation for inline svg fill-opacity SMIL animations lead to DoS

Status: NEW

(Core :: DOM: Security, defect, P3)

Description

[Matvey Kiselyov]

Attachment: response

Steps to reproduce the problem:

1. Send get request and receive response with csp style-src policy set and non-empty report-uri, containing html page with inline svg, having fill-opacity* attribute.

What is the expected behavior?
Opacity should change back and forth, like in the case without csp header. Moreover, no report to a server should be sent at all(that's the behavior of google chrome, for example).

What went wrong?
Firefox starts to send infinite amount of POST requests(reports) without any delay at the specified report-uri, claiming that style-src policy was violated, leading to an out-of-memory situation in the host operating system and effectively DOS-ing original server.

Attachment: response to a get request which would trigger the bug.

Comment 1

[:Gijs (out for now; he/him)]

Attachment: Testcase

Here's a loadable testcase that shows the issue if you open the console. I copied the CSP into a meta tag; no reports are sent because report directives in meta tags don't work, but you can still see the devtools console flooding with CSP reports.

Comment 2

[:Gijs (out for now; he/him)]

Here's a partial stack (opt build so might be a little iffy in places) on Windows from nsCSPContext::AsyncReportViolation which is what ends up creating the console error and/or requests.

>	xul.dll!nsCSPContext::AsyncReportViolation(mozilla::dom::Element * aTriggeringElement, nsICSPEventListener * aCSPEventListener, nsIURI * aBlockedURI, nsCSPContext::BlockedContentSource aBlockedContentSource, nsIURI * aOriginalURI, const nsTSubstring<char16_t> & aViolatedDirective, unsigned int aViolatedPolicyIndex, const nsTSubstring<char16_t> & aObserverSubject, const nsTSubstring<char16_t> & aSourceFile, const nsTSubstring<char16_t> & aScriptSample, unsigned int aLineNum, unsigned int aColumnNum) Line 1511	C++
 	xul.dll!nsCSPContext::reportInlineViolation(nsIContentSecurityPolicy::CSPDirective aDirective, mozilla::dom::Element * aTriggeringElement, nsICSPEventListener * aCSPEventListener, const nsTSubstring<char16_t> & aNonce, const nsTSubstring<char16_t> & aContent, const nsTSubstring<char16_t> & aViolatedDirective, unsigned int aViolatedPolicyIndex, unsigned int aLineNumber, unsigned int aColumnNumber) Line 527	C++
 	xul.dll!nsCSPContext::GetAllowsInline(nsIContentSecurityPolicy::CSPDirective aDirective, const nsTSubstring<char16_t> & aNonce, bool aParserCreated, mozilla::dom::Element * aTriggeringElement, nsICSPEventListener * aCSPEventListener, const nsTSubstring<char16_t> & aContentOfPseudoScript, unsigned int aLineNumber, unsigned int aColumnNumber, bool * outAllowsInline) Line 590	C++
 	xul.dll!nsStyleUtil::CSPAllowsInlineStyle(mozilla::dom::Element * aElement, mozilla::dom::Document * aDocument, nsIPrincipal * aTriggeringPrincipal, unsigned int aLineNumber, unsigned int aColumnNumber, const nsTSubstring<char16_t> & aStyleText, nsresult * aRv) Line 326	C++
 	xul.dll!mozilla::SMILCSSValueType::ValueFromString(nsCSSPropertyID aPropID, mozilla::dom::Element * aTargetElement, const nsTSubstring<char16_t> & aString, mozilla::SMILValue & aValue, bool * aIsContextSensitive) Line 459	C++
 	xul.dll!mozilla::SMILCSSProperty::ValueFromString(const nsTSubstring<char16_t> & aStr, const mozilla::dom::SVGAnimationElement * aSrcElement, mozilla::SMILValue & aValue, bool & aPreventCachingOfSandwich) Line 85	C++
 	xul.dll!mozilla::SMILValueParser::Parse(const nsTSubstring<char16_t> & aValueStr) Line 506	C++
 	[Inline Frame] xul.dll!mozilla::SMILParserUtils::ParseValuesGeneric(const nsTSubstring<char16_t> & aSpec, mozilla::SMILParserUtils::GenericValueParser & aParser) Line 547	C++
 	[Inline Frame] xul.dll!mozilla::SMILParserUtils::ParseValues(const nsTSubstring<char16_t> & aSpec, const mozilla::dom::SVGAnimationElement * aSrcElement, const mozilla::SMILAttr & aAttribute, FallibleTArray<mozilla::SMILValue> & aValuesArray, bool & aPreventCachingOfSandwich) Line 535	C++
 	xul.dll!mozilla::SMILAnimationFunction::GetValues(const mozilla::SMILAttr & aSMILAttr, FallibleTArray<mozilla::SMILValue> & aResult) Line 716	C++
 	xul.dll!mozilla::SMILAnimationFunction::ComposeResult(const mozilla::SMILAttr & aSMILAttr, mozilla::SMILValue & aResult) Line 193	C++
 	xul.dll!mozilla::SMILCompositor::ComposeAttribute(bool & aMightHavePendingStyleUpdates) Line 96	C++
 	xul.dll!mozilla::SMILAnimationController::DoSample(bool aSkipUnchangedContainers) Line 412	C++
 	xul.dll!mozilla::SMILTimeContainer::Sample() Line 162	C++
 	xul.dll!nsRefreshDriver::Tick(mozilla::layers::BaseTransactionId<mozilla::VsyncIdType> aId, mozilla::TimeStamp aNowTime, nsRefreshDriver::IsExtraTick aIsExtraTick) Line 2469	C++
 	[Inline Frame] xul.dll!nsRefreshDriver::EnsureTimerStarted::<lambda_2>::operator()() Line 1741	C++
 	xul.dll!mozilla::detail::RunnableFunction<`lambda at layout/base/nsRefreshDriver.cpp:1736:15'>::Run() Line 532	C++
 	xul.dll!mozilla::RunnableTask::Run() Line 468	C++

Looks like for SMIL content like this:

      <animate attributeName="fill-opacity" from="1" to="1" begin="0s" dur="0.8s" values="1;.5;1" calcMode="linear"

is responsible here, and presumably we should somehow avoid reporting for the same combination of attribute and value.

Christoph/Dan, do you know (who knows) how this should work?

Comment 3

[Daniel Holbert [:dholbert]]

(In reply to :Gijs (he/him) from comment #2)

Christoph/Dan, do you know (who knows) how this should work?

I don't know what the precise semantics/expectations are around the CSP-violation reporting here; but it seems like we should only report once per animation element (once per SVGAnimationElement * aSrcElement in Gijs' backtrace).

Maybe we could set a state bit on that element, at the moment we report; and then we would check the bit before we report again? (I guess we could clear the bit if animation-related attributes change, though maybe it's fine if we just leave the bit set and don't bother re-reporting for that same element, if the CSP requirements allow for that.)

Comment 4

[Matvey Kiselyov]

(In reply to Daniel Holbert [:dholbert] from comment #3)

(In reply to :Gijs (he/him) from comment #2)

Christoph/Dan, do you know (who knows) how this should work?

I don't know what the precise semantics/expectations are around the CSP-violation reporting here; but it seems like we should only report once per animation element (once per SVGAnimationElement * aSrcElement in Gijs' backtrace).

Do we actually need report any csp violation here? As far as i can tell, no external sources are referenced in the example attached, so the report might be surprising for a site administrator. Moreover, when analyzing traffic for report-only version, i can't see any get request to an external site.

Comment 5

[Daniel Holbert [:dholbert]]

(In reply to Matvey Kiselyov from comment #4)

Do we actually need report any csp violation here? As far as i can tell, no external sources are referenced in the example attached

Yes, we do.

You're right that there's no external source referenced; it's a CSP violation due to being considered a version of "unsafe-inline" category of styles.

See this for more:
https://content-security-policy.com/examples/allow-inline-style/

In Firefox at least, SVG Animations of CSS properties are considered as part of the "unsafe inline" category, since (like <style>...</style> elements) they're elements in the same page that directly contain markup to change the styles of some other piece of content on the page.

Comment 6

[Daniel Holbert [:dholbert]]

[removing "false negative" from title, for clarity, since it's not a false negative per comment 5. There is a real CSP violation, and the issue is just that we're reporting it over and over again.]

Comment 7

[Daniel Veditz [:dveditz]]

The DOS is really under the control of the site itself: doesn't need to be a hidden bug. An intentionally bad site could also have a small script that keeps generating HTML elements with inline style so this isn't making the attack any easier, it's just making it an easier-to-make problem for your own site.

Comment 8

[Daniel Veditz [:dveditz]]

Not sure how to fix this... maybe store something in the element that we've already reported it? Don't report if there's inline style being changed to inline style?