Closed Bug 1456378 Opened 7 years ago Closed 2 years ago

privacy.resistFingerprinting breaks image cropping in Expensify

Categories

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

defect

Tracking

()

RESOLVED WONTFIX

People

(Reporter: francois, Unassigned)

References

(Blocks 2 open bugs)

Details

(Whiteboard: [domsecurity-backlog1][fingerprinting][fp-triaged])

Steps: 0. Set privacy.resistFingerprinting = true in about:config. 1. Upload photo receipt to Expensify.com and attach it to an expense. 2. Click on the expense and crop the photo. Expected: The image gets cropped. Actual: The image is replaced with a blank one. Turning off privacy.resistFingerprinting and reloading the page fixes the problem, though the original receipts are gone and need to be uploaded again.
Priority: -- → P3
Whiteboard: [domsecurity-backlog1]
This sounds like a canvas issue. I would have expected it to generate a permission prompt; does it?
Whiteboard: [domsecurity-backlog1] → [domsecurity-backlog1][fingerprinting-breakage]
> I would have expected it to generate a permission prompt; does it? I repeated the test in a fresh profile and I can confirm that there is no prompt. It's just silently broken.
Whiteboard: [domsecurity-backlog1][fingerprinting-breakage] → [domsecurity-backlog1][fingerprinting]
Assignee: nobody → tihuang
Priority: P3 → P2
Whiteboard: [domsecurity-backlog1][fingerprinting] → [domsecurity-backlog1][fingerprinting][fp-triaged]

It doesn't show the door hanger of the canvas prompt, but it shows the canvas icon. This means the canvas auto-blocking been triggered in this case. But it isn't right since there is a user interaction involves, one click, in this case. So, the canvas auto-blocking should not happen. I am digging into it.

It turns out that the canvas prompt won't show if the extraction happens at the end of event loop. In other words, using setTimeout() to enforce the extraction happens at the end of event loop. In this case, even though the extraction is triggered by the click, but the canvas prompt still won't show. Here is a POC: https://artines1.github.io/canvas/index.html

It is because EventStateManager::IsHandlingUserInput() will report false in this case. So, my question is, is this the desired behavior when we implemented the canvas extraction auto-blocking?

Flags: needinfo?(tom)

(In reply to Tim Huang[:timhuang] from comment #4)

It turns out that the canvas prompt won't show if the extraction happens at the end of event loop. In other words, using setTimeout() to enforce the extraction happens at the end of event loop. In this case, even though the extraction is triggered by the click, but the canvas prompt still won't show. Here is a POC: https://artines1.github.io/canvas/index.html

It is because EventStateManager::IsHandlingUserInput() will report false in this case. So, my question is, is this the desired behavior when we implemented the canvas extraction auto-blocking?

I'm not sure =) We intended to break trackers that ran by default; responding to user input was an evolution of "Block anything that fires in the first N seconds of page load".

To fix this situation; we could (in theory at least) show the prompt if the extraction is triggered as a result of a settimeout/setinterval - but if trackers (inadvertently) use that technique to do their tracking; we might hit a lot of prompts. However the situation where triggered caller is third party is always blocked[0] and that seems like it would be more common for trackers.

[0] In fact; it's always blocked without showing the icon or granting the user the ability to override it; which seems certain it would break something on the internet. Maybe this is something we could/should fix in some way? Although I don't see a way to fix it without making it more complicated or weakening protection.

So I think the answer here is "We should show the doorhanger is the action is taken in response to settimeout/setinterval; browse the web, and see if we trigger the doorhanger in places we're not really interacting the with the page." However; determining if we're the result of setTimeout/setInterval is probably not as easy as detecting if we're handling user input (where we found a wonder function) so that would make implementation much harder; and since we're pursuing uniform canvas rendering we should backburner that work until we know more about the rendering.

Flags: needinfo?(tom)

After thinking this more, maybe we don't have to change the behavior. The current situation is "Prompt the user if the canvas extraction is not happening during user interactions." And this sounds correct to me. We should consider any extractions that happened while user is not interacting with the browser are potentially malicious in RFP mode.

Of course, there will be situation like this. However, users can still see the canvas icon in this case. So they can use their judgment to decide if they want to grant or not.

What do you think, Tom?

Flags: needinfo?(tom)

(In reply to Tim Huang[:timhuang] from comment #6)

After thinking this more, maybe we don't have to change the behavior. The current situation is "Prompt the user if the canvas extraction is not happening during user interactions." And this sounds correct to me. We should consider any extractions that happened while user is not interacting with the browser are potentially malicious in RFP mode.

Of course, there will be situation like this. However, users can still see the canvas icon in this case. So they can use their judgment to decide if they want to grant or not.

What do you think, Tom?

Eh; I think you're focusing too much on the white and black of "user interaction" and not on the overall goal of "Block canvas extractions that are intended to fingerprint users and allow* canvas extractions that are intended to provide the user website functionality. However; we can't outright allow them because we'd never be able to see a difference; so we prompt instead of allowing.

This situation is one that's providing website functionality and not tracking. So we want to prompt.

As far as heuristics go, we can be more strict and not prompt for trackers while breaking more functionality (like we do here) or we can be looser, possibly prompt for more trackers but not break functionality. I'd probably err on the side of being looser; if we think we won't be triggering on an unacceptable amount of trackers.

Flags: needinfo?(tom)

(In reply to Tom Ritter [:tjr] from comment #7)

(In reply to Tim Huang[:timhuang] from comment #6)

After thinking this more, maybe we don't have to change the behavior. The current situation is "Prompt the user if the canvas extraction is not happening during user interactions." And this sounds correct to me. We should consider any extractions that happened while user is not interacting with the browser are potentially malicious in RFP mode.

Of course, there will be situation like this. However, users can still see the canvas icon in this case. So they can use their judgment to decide if they want to grant or not.

What do you think, Tom?

Eh; I think you're focusing too much on the white and black of "user interaction" and not on the overall goal of "Block canvas extractions that are intended to fingerprint users and allow* canvas extractions that are intended to provide the user website functionality. However; we can't outright allow them because we'd never be able to see a difference; so we prompt instead of allowing.

This situation is one that's providing website functionality and not tracking. So we want to prompt.

As far as heuristics go, we can be more strict and not prompt for trackers while breaking more functionality (like we do here) or we can be looser, possibly prompt for more trackers but not break functionality. I'd probably err on the side of being looser; if we think we won't be triggering on an unacceptable amount of trackers.

I think you are right here. I was indeed focusing too much on whether or not the canvas extraction is triggered by user interactions. And we should do what you mentioned in comment 5.

Unassign myself because I am no longer actively working on this.

Assignee: tihuang → nobody
Severity: normal → S3

Realistically, we will not be fixing this, and are pursuing software rendering for canvas.

Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.