Closed Bug 1141386 Opened 5 years ago Closed 5 years ago

[EME] gmp-clearkey's base64decode doesn't handle padding properly

Categories

(Core :: Audio/Video, defect)

39 Branch
x86_64
Windows 8.1
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla39
Tracking Status
firefox38 --- fixed
firefox39 --- fixed

People

(Reporter: cpearce, Assigned: cpearce)

References

(Blocks 1 open bug)

Details

Attachments

(2 files)

My last change to gmp-clearkey's base64 key/Id decode didn't handle padding properly. It assumed that the input key was always 22 bytes long, i.e. that JS always stripped the pading '=' characters from base64 encoded key/Ids. This is not always the case.
Attached patch PatchSplinter Review
Strip trailing padding, and only then validate the length of the key/Id for base64 decode.

We should get this fix landed ASAP. I'm going to write a gtest so we don't regress...
Attachment #8574982 - Flags: review?(edwin)
Add a gtest, so we don't regress this...
Attachment #8575028 - Flags: review?(edwin)
Comment on attachment 8574982 [details] [diff] [review]
Patch

Approval Request Comment
[Feature/regressing bug #]: EME
[User impact if declined]: EME won't work on some EME sites
[Describe test coverage new/current, TreeHerder]: There's a test in the next patch. Performed local testing too.
[Risks and why]: Low; isolated to EME, and I've tested the target site.
[String/UUID change made/needed]: None.
Attachment #8574982 - Flags: approval-mozilla-aurora?
Comment on attachment 8575028 [details] [diff] [review]
Patch 2: Add gtest for gmp-clearkey's base64 decode

Approval Request Comment
[Feature/regressing bug #]: EME
[User impact if declined]: EME won't work on some EME sites
[Describe test coverage new/current, TreeHerder]: This is the test for the fix to EME in the preceeding patch.
[Risks and why]: Low; isolated to EME, and I've tested the target site.
[String/UUID change made/needed]: None.
Attachment #8575028 - Flags: approval-mozilla-aurora?
Flags: needinfo?(cpearce)
Flags: needinfo?(cpearce)
Attachment #8575028 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Attachment #8574982 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Flags: needinfo?(cpearce)
You need to log in before you can comment on or make changes to this bug.