1287863 - Assertion failure: ok || !(aPresContext && aStyleContext) in [@nsRuleNode::ComputeColor]

Status: RESOLVED WORKSFORME

(Core :: CSS Parsing and Computation, defect, P3)

Description

[Tyson Smith [:tsmith]]

Attachment: log.txt

Found with :truber's css-color grammar work while fuzzing canvas.

Assertion failure: ok || !(aPresContext && aStyleContext), at /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/layout/style/nsRuleNode.cpp:10619

Test case:
<canvas id='i0'></canvas>
<script>
var c=document.getElementById('i0').getContext('2d');
c.strokeStyle='-moz-win-mediatext';
//c.fillStyle='-moz-mac-focusring'; // also triggers assertion
</script>

Comment 1

[Vincent Liu[:vliu]]

Attachment: WIP.patch

Hi David,

About the assertion for '-moz-win-mediatext', I'd tried to enable sUseStandinsForNativeColors to stop the crash. Please see the attached WIP.
The reason why I did this is because I saw Gecko has defined this attribute in [1].

[1]: https://dxr.mozilla.org/mozilla-central/source/widget/nsXPLookAndFeel.cpp#650

Could you please give my any comment if the WIP is not considering anything others? Thanks.

Comment 2

[Dave Huseby [:huseby]]

So this feature is a privacy enhancement to prevent CSS named color fingerprinting.  This patch came from the Tor Project and it should be disabled by default.  When using stand-in colors is enabled, we're telling Firefox to use the Windows colors instead of the native colors for the system.  Maybe your assertion is wrong?

Comment 3

[Vincent Liu[:vliu]]

Attachment: logs.log

(In reply to Dave Huseby [:huseby] from comment #2)
> So this feature is a privacy enhancement to prevent CSS named color
> fingerprinting.  This patch came from the Tor Project and it should be
> disabled by default.  When using stand-in colors is enabled, we're telling
> Firefox to use the Windows colors instead of the native colors for the
> system.  Maybe your assertion is wrong?


Hi Dave,

The attached log file is the issue I saw. I'd see this issue on MAC OS. Could you please comment for me if I miss something? Thanks

Comment 4

[Dave Huseby [:huseby]]

So the use standins for native colors boolean is only enabled when the user manually sets the pref to hide the native colors of the operating system from JS scripts using named CSS colors to figure out which OS the user is using.  It is an active fingerprinting vector.  Enabling it by default is not the correct solution.  The assertion is being caused by incorrect *default behavior* code.  I think you've looped me in because it looks like my standins patch caused this.  I don't think that is the case.  I would investigate why the assertion is failing.  I'd step through the code and see what's going on.

Comment 5

[Vincent Liu[:vliu]]

(In reply to Dave Huseby [:huseby] from comment #4)
> So the use standins for native colors boolean is only enabled when the user
> manually sets the pref to hide the native colors of the operating system
> from JS scripts using named CSS colors to figure out which OS the user is
> using.  It is an active fingerprinting vector.  Enabling it by default is
> not the correct solution.  The assertion is being caused by incorrect
> *default behavior* code.  I think you've looped me in because it looks like
> my standins patch caused this.  I don't think that is the case.  I would
> investigate why the assertion is failing.  I'd step through the code and see
> what's going on.

Really thanks for the great help to look into and welcome to take this bug if you want.

Comment 6

[Mats Palmgren (inactive)]

It seems to me this assertion is bogus and should be removed.
SetColor is expected to fail (return false) for the *-win-* and *-mac-* colors on Linux.

Comment 7

[Tyson Smith [:tsmith]]

Any update here? This issue is still getting hit very frequently by our fuzzer.

Comment 8

[Vincent Liu[:vliu]]

(In reply to Dave Huseby [:huseby] from comment #4)
> So the use standins for native colors boolean is only enabled when the user
> manually sets the pref to hide the native colors of the operating system
> from JS scripts using named CSS colors to figure out which OS the user is
> using.  It is an active fingerprinting vector.  Enabling it by default is
> not the correct solution.  The assertion is being caused by incorrect
> *default behavior* code.  I think you've looped me in because it looks like
> my standins patch caused this.  I don't think that is the case.  I would
> investigate why the assertion is failing.  I'd step through the code and see
> what's going on.

I would like to untake this bug according to Comment 5.

Comment 9

[Cameron McCormack (:heycam)]

nsRuleNode is gone now.