Skip to content

Security review #22

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

Closed
ngparker opened this issue May 15, 2015 · 7 comments
Closed

Security review #22

ngparker opened this issue May 15, 2015 · 7 comments

Comments

@ngparker
Copy link

[Intro: I am Nathan Parker of the Google Chrome Security Team]

I have some question and comments on the spec, and one general concern which is described in the first security scenario below. I look forward to your responses.

Questions

  • Will service workers get access and how will that persist?
  • How and why is “URL scope” different from a exact origin match? The API permission should be based on exact origin match.
  • When asking for permission, will it be always for general Web NFC access or for a subset of features (read vs read/write)?

Comments

  • Non HTTPS sites should get no access the API. I think the NOTE about it should be a requirement in the spec.
  • Should only allow API access on top-level frames
  • Should always require permission before reading arbitrary tags.
  • For Bluetooth and Wifi handover, the user should have to grant access to the secondary API and must be able to properly understand what they’re granting.
  • A tag containing JSON shouldn’t be eval()’d. It wasn't clear to me how that was parsed.
  • Send() method:  The “platform specific timeout” should be bounded so the user doesn’t accidentally write to the wrong device many minutes later, potentially leaking data.

Security scenarios to address

  1. A malicious Web NFC tag could send an arbitrary payload to a website via a user’s device, along with a user’s existing web credentials.  The webapp and server would need to treat that payload as completely untrusted.  The spec should address this in some way.  My concern is that developers will make mistakes here and end up exposed to attacks similar to persistent XSS.
    Some potential directions to take this: Suggest rules for handling that payload safely, provide best-practice methods for doing so, provide a sanitization/validation function.  The webapp’s server could even cryptographically sign a payload before writing it to a tag so the contents could later be verified.
  2. A Web NFC tag (say, embedded in restaurant table) should not ever trigger a user’s device to navigate to a website without asking permission, unless the site is in the foreground and has been granted permission.  Otherwise, this would leak the user’s location unexpectedly.
@zolkis
Copy link
Contributor

zolkis commented May 15, 2015

Thank you for the review, Nathan. I will update the spec considering the comments and scenarios.
For the added scenarios, you can also edit the Security doc and submit a PR.
About the questions:

  • service workers are not used ATM; see also service workers, protocol handlers #17. In principle, a page handling NFC should be visible and in focus.
  • URL scope: indeed needs more thinking; could use Clarify scope matching #15 for discussion. Suggestions are welcome (understood that you'd prefer exact origin match).
  • permissions: in principle there could be separate permission on tag write, tag read, and peer communication, but with the current API it's only for using NFC as a whole.

zolkis added a commit to zolkis/web-nfc that referenced this issue May 18, 2015
@anssiko
Copy link
Member

anssiko commented Jun 18, 2015

@ngparker Should we require a secure context? The Secure Contexts spec defines a set of requirements that addresses some of your concerns that are more generic in nature.

@zolkis zolkis mentioned this issue Aug 11, 2015
kenchris pushed a commit that referenced this issue Aug 28, 2015
#22: addressed review comments, added security threats
zolkis added a commit to zolkis/web-nfc that referenced this issue Sep 5, 2015
…ad events. Fixed terminology related issues. Renamed send() to pushMessage(). Add timeout to push options. Issues addressed: w3c#2, w3c#3, w3c#22, w3c#26, w3c#28, w3c#30, w3c#31, w3c#32, w3c#33, w3c#35, w3c#36, w3c#38, w3c#39, w3c#40.
@kenchris
Copy link
Contributor

kenchris commented Nov 9, 2015

Regarding parsing JSON. That will not be done using eval but using the JSON.parse algorithm, which is considered safe.

http://www.ecma-international.org/ecma-262/6.0/#sec-json.parse
https://blog.whitehatsec.com/handling-untrusted-json-safely/

The result will be returned in a dictionary. Does this solve your worries @ngparker ?

@OlivierGre
Copy link

Hi,
This thread is pretty old but I have a comment about this:

Security scenarios to address
2. A Web NFC tag (say, embedded in restaurant table) should not ever trigger a user’s device to navigate to a website without asking permission, unless the site is in the foreground and has been granted permission. Otherwise, this would leak the user’s location unexpectedly.

In my opinion, you have such risk with an ordinary NFC tag:
If someone embeds a NFC tag in a restaurant table (or put it under the tablecloth), the tag can open the browser and launch a malicious URI without asking the persmission of the user.
This is possible with Android phones but not with IPhones (on IPhones, NFC should be started by the user and it works only a few second).

So, for Web NFC, why do we want to prevent something that is in fact possible with any Android phone?

Regards
Olivier

@kenchris
Copy link
Contributor

I am not sure how much of an issue that actually is. Generally loading web sites is not dangerous due to the sandbox of the web, but you can of course encode specific URLs like ?location=showonsecondstreet and that way somehow track where the user is, but it might not really be a big issue.

Actually as Android can already launch the browser for URLs then it makes little sense for this spec to conflict with it and try to do the same. On the other hand it would be nicer to actually launch the site and then somehow still be able to get the info read from the NFC from the site, a bit like Web Bluetooth discussed having the navigator.bluetooth.referringDevice when launched from a bluetooth beacon.

@kenchris
Copy link
Contributor

Questions

  • Will service workers get access and how will that persist?

SWs are not supported

  • How and why is “URL scope” different from a exact origin match? The API permission should be based on exact origin match.

Done

  • When asking for permission, will it be always for general Web NFC access or for a subset of features (read vs read/write)?

Read and write are treated separately

Comments

  • Non HTTPS sites should get no access the API. I think the NOTE about it should be a requirement in the spec.

Done, Feature is HTTPS only

  • Should only allow API access on top-level frames

Done

  • Should always require permission before reading arbitrary tags.

Done: https://w3c.github.io/web-nfc/#dfn-obtain-reading-permission

  • For Bluetooth and Wifi handover, the user should have to grant access to the secondary API and must be able to properly understand what they’re granting.

Not specced

  • A tag containing JSON shouldn’t be eval()’d. It wasn't clear to me how that was parsed.

Done, this is using https://infra.spec.whatwg.org/#parse-json-from-bytes - see https://w3c.github.io/web-nfc/#the-ndef-parsing-algorithm

  • Send() method:  The “platform specific timeout” should be bounded so the user doesn’t accidentally write to the wrong device many minutes later, potentially leaking data.

Not done, filed #208

Security scenarios to address

  1. A malicious Web NFC tag could send an arbitrary payload to a website via a user’s device, along with a user’s existing web credentials.  The webapp and server would need to treat that payload as completely untrusted.  The spec should address this in some way.  My concern is that developers will make mistakes here and end up exposed to attacks similar to persistent XSS.

NDEF content should always be treated as not-trusted, and the API allows you to easily make sure that you only get content that was written by a specific domain (like your own)

Some potential directions to take this: Suggest rules for handling that payload safely, provide best-practice methods for doing so, provide a sanitization/validation function.  The webapp’s server could even cryptographically sign a payload before writing it to a tag so the contents could later be verified.

URLs, strings and JSON is already validated by platform parsing rules (as defined in INFRA spec etc). If custom binary data is written, that can only be understood if you know how it was written, so the authors would need to do their own validation.

  1. A Web NFC tag (say, embedded in restaurant table) should not ever trigger a user’s device to navigate to a website without asking permission, unless the site is in the foreground and has been granted permission.  Otherwise, this would leak the user’s location unexpectedly.

That is outside of the NFC spec, as that is handled at platform level, for instance Android

@anssiko
Copy link
Member

anssiko commented Feb 26, 2019

@ngparker, thank you for the security review. The group has now addressed your feedback, except #208 that will be addressed separately. Should you have any concerns or further comments, feel free to reopen this issue.

@anssiko anssiko closed this as completed Feb 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants