Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Discuss or address possible implementor privacy and security relevant concerns #171

Open
pes10k opened this issue Dec 5, 2024 · 9 comments
Labels
privacy-needs-resolution Issue the Privacy Group has raised and looks for a response on.

Comments

@pes10k
Copy link

pes10k commented Dec 5, 2024

This issue is being filed as part of the requested PrivacyWG HR review (w3cping/privacy-request#147)

My understanding from MDN is that some implementors (e.g., Safari and Chrome) do no send beforeinput events in some cases for privacy and security reasons. For example, beforeinput events aren't sent for password managers.

If my understanding is correct, it would be good to at least provide guidance in the spec to implementors about when beforeinput events could have privacy and security risks.

Even better, if possible, to address these privacy-and-security-risking cases into the spec, either by normatively specifying what cases user agents shouldn't fire beforeinput events, and/or by modifying the specified behavior so that the events can be fired in these cases w/o security and privacy risk.

@pes10k pes10k added the privacy-needs-resolution Issue the Privacy Group has raised and looks for a response on. label Dec 5, 2024
@johanneswilm johanneswilm added the Agenda+ Queue this item for discussion at the next WG meeting label Dec 8, 2024
@johanneswilm
Copy link
Contributor

johanneswilm commented Dec 8, 2024

@pes10k : Are you referring to this note:

Not every user modification results in beforeinput firing. Also the event may fire but be non-cancelable. This may happen when the modification is done by autocomplete, by accepting a correction from a spell checker, by password manager autofill, by IME, or in other ways. The details vary by browser and OS. To override the edit behavior in all situations, the code needs to handle the input event and possibly revert any modifications that were not handled by the beforeinput handler.
https://developer.mozilla.org/en-US/docs/Web/API/Element/beforeinput_event

And this discussion: https://bugzilla.mozilla.org/show_bug.cgi?id=1673558 ?

As far as I can tell, there is nothing privacy related in that issue/comment. Testing in Chromium, both with and without input events, JavaScript is able to read the values of input[type=text] and input[type=password elements even if that field was filled by a password manager. This could for example be done at the time of form submission. It's also possible to change the values with JavaScripty after the value has been set with a password manager [1].

However, if a password manager is used, then no beforeinput event is triggered in Chromium, which is not correct according to the spec.

[1] Tested on Chromium with this code on a login page:

const passwordInput = document.querySelector('input[type=password]')
let password = passwordInput.value
setInterval(
	() => {
		if (passwordInput.value !== password) {
			console.log(`New password: ${passwordInput.value}`)
			passwordInput.value = password
		}
	}, 
	1000
)

@pes10k
Copy link
Author

pes10k commented Dec 9, 2024

Yep, those are the MDN comment and bugzilla comments I was referring to.

However, if a password manager is used, then no beforeinput event is triggered in Chromium, which is not correct according to the spec.

Understood that Chromium and Webkit diverge from the spec here. My question in this issue is:

  1. are Chromium and Webkit diverging from the spec for privacy and security related reasons (such as, because of possible confusion with a page intercepting a password manager form fill, in a way that made it difficult for the user to understand what was going on)?
  2. if so, do the Chromium/Webkit concerns suggest improvements that should be folded into the spec before the spec transitions, and/or non-normative text in the spec describing the implementor concerns that future implementors should keep in mind (things like tradeoffs)

@johanneswilm
Copy link
Contributor

johanneswilm commented Dec 10, 2024

@pes10k OK, understood. We'll await their feedback.

From what I have seen hitherto, this just seems like a bug in Chromium/Webkit.

As far as I can tell, there is no reason why beforeinput events should not be triggered when password managers are used. Even without it, a malicious page is able to read out the password. And a site that wants to annoy the user is able to change the password that the password manager has inserted before the form has been submitted.

I don't see how a website cancelling the beforeinput event and ignoring the input from a password manager would be any different than a site doing the same with the changes that come from the built-in spell checker. In both cases, it's annoying to the user, but if websites insist on doing it, it's impossible to prevent them from doing it by leaving out the beforeinput event. Additionally, leaving out the beforeinput event is highly annoying to JS authors who rely on the beforeinput event and mean that they really cannot use it and have to go back to the old way of using other ways of checking for changes (for example by looking at the value at an interval).

@johanneswilm
Copy link
Contributor

johanneswilm commented Dec 12, 2024

from meeting minutes (2024-12-12):

let's resolve for now — just bug in the implementation
(no privacy risk — WK and blink will investigate if this is intentional and file bugs if needed)

@johanneswilm johanneswilm removed the Agenda+ Queue this item for discussion at the next WG meeting label Dec 12, 2024
@johanneswilm
Copy link
Contributor

@pes10k Initial feedback is that this is just a bug in Safari/Chromium. Further investigations may change this preliminary conclusion.

@dandclark
Copy link
Contributor

Filed https://issues.chromium.org/issues/383841338 to track this in Chromium.

@pes10k
Copy link
Author

pes10k commented Dec 16, 2024

Thanks @johanneswilm and @dandclark for following up and filing that issue. Do you know if there is an equivalent issue for WebKit (or equivalent place that conversation is happening)?

I appreciate you all running this down, and I take your point, it might end up that these implementation decisions end up being bugs, or intentional decisions unrelated to privacy concerns. But would just like to get that confirmed and documented if possible (given the relationship between this functionality and critical security and privacy tools like password managers)

@johanneswilm
Copy link
Contributor

johanneswilm commented Dec 16, 2024

I believe @whsieh will file a similar bug with Webkit.

But would just like to get that confirmed and documented if possible (given the relationship between this functionality and critical security and privacy tools like password managers)

So far we have not been able to find anyone in Webkit/Chromium who says that it was a deliberate choice to leave out beforeinput events for password managers, and we have spoken to those currently working on these technologies. We were also not able to figure out how leaving out this beforeinput event would protect privacy or what privacy concerns it would address. In case we end up finding someone who can confirm that it was a deliberate choice and that it is due to privacy concerns, I agree that it makes sense to document this.

But as that does not seem to be the case right now, do you still think there is something to document @pes10k ? How much more effort do you think we need to put into trying to find someone who might say that there is a privacy concern there?

@whsieh
Copy link

whsieh commented Dec 16, 2024

I believe @whsieh will file a similar bug with Webkit.

Filed: https://bugs.webkit.org/show_bug.cgi?id=284769

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
privacy-needs-resolution Issue the Privacy Group has raised and looks for a response on.
Projects
None yet
Development

No branches or pull requests

4 participants