Skip to content
New issue

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

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

Already on GitHub? # to your account

WebCodecs 2021-04-14 #1

Closed
chcunningham opened this issue Apr 14, 2021 · 8 comments
Closed

WebCodecs 2021-04-14 #1

chcunningham opened this issue Apr 14, 2021 · 8 comments
Labels
FPWD First Public Working Draft published. pending This issue needs to get a reviewer assigned to it REVIEW REQUESTED

Comments

@chcunningham
Copy link

Other comments:

  • Please reach out to me via email (chcunningham@chromium.org) ASAP to setup a virtual meeting (I'll loop in co-editors). The spec is dense with media domain knowledge and I expect a fair bit of in depth discussion.
  • Author feedback from the Chrome origin trial has been very positive and these users are eager to see it ship. This API has generated considerable user excitement (twitter, discourse)

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

@chcunningham chcunningham added FPWD First Public Working Draft published. REVIEW REQUESTED pending This issue needs to get a reviewer assigned to it labels Apr 14, 2021
@chcunningham
Copy link
Author

Re: timeline, please see my clarifications in the parallel privacy review discussion
w3cping/privacy-request#38 (comment)

@samuelweiler
Copy link
Member

Review requested from @jsalowey

@jsalowey
Copy link

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

@jsalowey
Copy link

jsalowey commented Jun 6, 2021

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.

  1. 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.

  1. 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.

  1. 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.

  1. 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:

  • Suggest that implementations focus additional fuzz testing on their implementations where ordering of operations or input validation is enforced or difficult to manipulate in the current mechanisms of access.

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
Copy link
Author

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
Copy link

jsalowey commented Jun 7, 2021

@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
Copy link

aboba commented Jun 8, 2021

@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
Copy link

padenot commented Jun 8, 2021

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.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
FPWD First Public Working Draft published. pending This issue needs to get a reviewer assigned to it REVIEW REQUESTED
Projects
None yet
Development

No branches or pull requests

6 participants