w3c / security-request

Horizontal review requests will be made via issues in this repo.
4 stars 4 forks source link

WebCodecs 2021-04-14 #1

Open chcunningham opened 3 years ago

chcunningham commented 3 years ago

Other comments:

FYI co-editors: @padenot (Mozilla) and @aboba (Microsoft)

chcunningham commented 3 years ago

Re: timeline, please see my clarifications in the parallel privacy review discussion https://github.com/w3cping/privacy-request/issues/38#issuecomment-820087059

samuelweiler commented 3 years ago

Review requested from @jsalowey

jsalowey commented 3 years ago

I'll try to have comments in by the end of the week.

jsalowey commented 3 years ago

Here are the comments I discussed with @chcunningham:

  1. Has any similar API been deployed, such as a proprietary version of this specification? Have flaws been uncovered in these deployments?

    Many similar APIs, basically one per OS + various open source libs. Probably all have had security bugs. Most famous is the "stagefright" bug (Android's media API). Sandboxing addresses.

  2. Many interfaces to the codecs are accessible through other means now. It seems it may be possible to highlight interfaces that may be exposed that are typically behind a layer of input validation that will now be bypassed.

    Input validation isn't formally required for most of the existing APIs. Some implementations may > inspect things like frame headers, but the bulk content is simply passed through to the underlying codec.

  3. In addition to race conditions there may be ordering flaws uncovered by this API (but perhaps these are effectively the same thing in this context).

    Yes, new race conditions could be exposed that require specific ordering of codec controls. Existing APIs (e.g.

  4. The privacy considerations make reference that the codec may be one bit of information. How is this quantity arrived at? It seems that it could be more than 1 bit.

    Sorry, this is poor phrasing on my part. I just meant "bit" in the generic sense. I'll reword that.

  5. Are there other implementation choices that could be made to protect from codec vulnerabilities, such as process separation, sandboxing etc.

    Yes. It is common for implementers to isolate and sandbox codec execution (particularly when involving underlying OS APIs) as part of their existing media stacks. The security section encourages this mitigation.

In general I do not think this API has taken care not to add significant surface area beyond what is already provided by the the current accessibility of codecs through media on the web. I would suggest that there be some additions to the security considerations:

Also I think the privacy considerations section should avoid the use of the specific bit word since its unclear how many bits that the codec information will leak.

@samuelweiler FYI

chcunningham commented 3 years ago

Thanks @jsalowey

In general I do not think this API has taken care not to add significant surface area beyond what is already provided by the the current accessibility of codecs through media on the web. I would suggest that there be some additions to the security considerations:

I'm guessing an extra 'not' snuck in to the sentence above? I thought you felt the opposite (i.e. not very concerned) after our discussion. LMK if I've got it backwards.

jsalowey commented 3 years ago

@chcunningham Yup, english is hard. It should read "In general I do think this API has taken care not to add significant surface area beyond what is already provided by the the current accessibility of codecs through media on the web.

aboba commented 3 years ago

@jsalowey @chcunningham

"In general I think this API has taken care not to add significant surface area beyond what is already provided by the the current accessibility of codecs through media on the web."

[BA] On the decoder side, there are other Web APIs (e.g. MSE) that have similar surface area, but on the encoder side, WebCodecs is unique. WebRTC API feeds a MediaStreamTrack to the encoder, but doesn't let the developer get in between, so as to allow arbitrary data to be fed to an encoder. I think WebCodecs encoding does add surface area compared with say, WebRTC and Media Recorder.

So additional fuzzing is probably a good idea, particularly when additional encoders (in sw or hw) are added.

padenot commented 3 years ago

WebRTC API feeds a MediaStreamTrack to the encoder, but doesn't let the developer get in between, so as to allow arbitrary data to be fed to an encoder.

Except with https://github.com/w3c/mediacapture-transform/ and MediaRecorder. Gecko will use the same underlying encoder and decoder code regardless of the high-level API, lots of fuzzing has been done already, and I expect a lot more.