Bug 189740

Summary: [EME] Add WebM sanitization
Product: WebKit Reporter: Yacine Bandou <bandou.yacine>
Component: MediaAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: calvaris, commit-queue, olivier.blin, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 189696    
Attachments:
Description Flags
Patch
calvaris: review+
Patch
none
Patch
none
Patch
none
Patch none

Yacine Bandou
Reported 2018-09-19 03:04:32 PDT
Add Webm sanitization.
Attachments
Patch (3.19 KB, patch)
2018-09-19 05:52 PDT, Yacine Bandou 		calvaris: review+
DetailsFormatted DiffDiff
Patch (3.17 KB, patch)
2018-09-19 06:17 PDT, Yacine Bandou 		no flags
DetailsFormatted DiffDiff
Patch (3.20 KB, patch)
2018-09-20 03:38 PDT, Yacine Bandou 		no flags
DetailsFormatted DiffDiff
Patch (1.80 KB, patch)
2018-09-20 08:58 PDT, Yacine Bandou 		no flags
DetailsFormatted DiffDiff
Patch (3.20 KB, patch)
2018-09-20 09:08 PDT, Yacine Bandou 		no flags
DetailsFormatted DiffDiff
Show Obsolete (4) View All Add attachment proposed patch, testcase, etc.
Yacine Bandou
Comment 1 2018-09-19 05:52:05 PDT
Created attachment 350106 [details] Patch
Yacine Bandou
Comment 2 2018-09-19 06:17:49 PDT
Created attachment 350108 [details] Patch
Xabier Rodríguez Calvar
Comment 3 2018-09-19 08:33:59 PDT
Comment on attachment 350106 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=350106&action=review > Source/WebCore/ChangeLog:10 > + see https://www.w3.org/TR/encrypted-media/#dom-mediakeysession-generaterequest Capital at the beginning, period at the end. > Source/WebCore/Modules/encryptedmedia/InitDataRegistry.cpp:160 > + if (buffer.isEmpty() || buffer.size() > kWebmMaxContentEncKeyIDSize) > + return std::nullopt; > + keyIDs.append(buffer.copy()); Instead of this, I'd try to sanitize and if the result is null, bailout. Otherwise, move the WTFMove the result inside the append
Yacine Bandou
Comment 4 2018-09-20 03:38:41 PDT
Created attachment 350184 [details] Patch
Olivier Blin
Comment 5 2018-09-20 04:06:22 PDT
Comment on attachment 350184 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=350184&action=review > Source/WebCore/Modules/encryptedmedia/InitDataRegistry.cpp:157 > + RefPtr<SharedBuffer> sanitazedBuffer = sanitizeWebM(buffer); You probably mean "sanitized"
WebKit Commit Bot
Comment 6 2018-09-20 04:22:07 PDT
Comment on attachment 350184 [details] Patch Clearing flags on attachment: 350184 Committed r236256: <https://trac.webkit.org/changeset/236256>
WebKit Commit Bot
Comment 7 2018-09-20 04:22:09 PDT
All reviewed patches have been landed. Closing bug.
Radar WebKit Bug Importer
Comment 8 2018-09-20 04:23:26 PDT
<rdar://problem/44636894>
Xabier Rodríguez Calvar
Comment 9 2018-09-20 06:15:52 PDT
(In reply to Olivier Blin from comment #5) > > Source/WebCore/Modules/encryptedmedia/InitDataRegistry.cpp:157 > > + RefPtr<SharedBuffer> sanitazedBuffer = sanitizeWebM(buffer); > > You probably mean "sanitized" Very true! Good catch. Sad that the patch landed already. Can you file a follow up please?
Yacine Bandou
Comment 10 2018-09-20 08:58:08 PDT
Reopening to attach new patch.
Yacine Bandou
Comment 11 2018-09-20 08:58:10 PDT
Created attachment 350208 [details] Patch
Yacine Bandou
Comment 12 2018-09-20 09:08:16 PDT
Created attachment 350213 [details] Patch
Yacine Bandou
Comment 13 2018-09-20 09:26:06 PDT
I pushed the attached patch 350208 by mistake, instead of pushing it on the bug 189789, I pushed it on this one. I pushed again the original patch of this bug for reference.
Note You need to log in before you can comment on or make changes to this bug.