Bug 188376 – Add CENC sanitization

• Status: RESOLVED
• Resolution: FIXED
• Component: Media

[Charlie Turner]

Add CENC sanitization

[Charlie Turner]

Created attachment 346700 [details]
Patch

[EWS Watchlist]

Comment on attachment 346700 [details]
Patch

Attachment 346700 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/8786298

New failing tests:
http/wpt/beacon/cors/cors-redirect-failure.html

[EWS Watchlist]

Created attachment 346703 [details]
Archive of layout-test-results from ews104 for mac-sierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104  Port: mac-sierra-wk2  Platform: Mac OS X 10.12.6

[Xabier Rodríguez Calvar]

Comment on attachment 346700 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=346700&action=review

I guess you also have to fix the ios builds

> Source/WebCore/Modules/encryptedmedia/InitDataRegistry.cpp:125
> +            Ref<SharedBuffer> keyIDBuffer = SharedBuffer::create(WTFMove(value));
> +            keyIDs.append(WTFMove(keyIDBuffer));

You can collapse these two lines, save the variable and the move.

> Source/WebCore/platform/graphics/iso/ISOProtectionSystemSpecificHeaderBox.cpp:2
> + * Copyright (C) 2018 Apple Inc. All rights reserved.

I guess copyright does not go to Apple here, right?

> Source/WebCore/platform/graphics/iso/ISOProtectionSystemSpecificHeaderBox.cpp:51
> +    auto systemID = buffer->slice(offset, offset + 16);
> +    if (!systemID)
> +        return false;
> +
> +    offset += 16;
> +
> +    m_systemID.resize(16);
> +    memcpy(m_systemID.data(), systemID->data(), 16);

I don't have a strong preference on this one but since you are copying the value and even when slicing does not copy the value in all cases, we might want to do the maths ourselves and avoid slicing. Same for the key ids and data.

> Source/WebCore/platform/graphics/iso/ISOProtectionSystemSpecificHeaderBox.h:2
> + * Copyright (C) 2018 Apple Inc. All rights reserved.

Ditto.

> Source/WebCore/platform/graphics/iso/ISOProtectionSystemSpecificHeaderBox.h:29
> +#include "ISOProtectionSystemSpecificHeaderBox.h"

Is this file including itself or did I miss any subtle difference?

[Charlie Turner]

Comment on attachment 346700 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=346700&action=review

>> Source/WebCore/platform/graphics/iso/ISOProtectionSystemSpecificHeaderBox.cpp:2
>> + * Copyright (C) 2018 Apple Inc. All rights reserved.
> 
> I guess copyright does not go to Apple here, right?

I don't know, I basically copied what Apple had already done and modified it for a new box, seemed a bit cheeky taking copyright of it, but I'm no lawyer.

>> Source/WebCore/platform/graphics/iso/ISOProtectionSystemSpecificHeaderBox.cpp:51
>> +    memcpy(m_systemID.data(), systemID->data(), 16);
> 
> I don't have a strong preference on this one but since you are copying the value and even when slicing does not copy the value in all cases, we might want to do the maths ourselves and avoid slicing. Same for the key ids and data.

Me neither, will see if anyone else objects to it, otherwise I'm inclined to leave it as-is.

>> Source/WebCore/platform/graphics/iso/ISOProtectionSystemSpecificHeaderBox.h:29
>> +#include "ISOProtectionSystemSpecificHeaderBox.h"
> 
> Is this file including itself or did I miss any subtle difference?

Oops, that's a mistake, will fix.

[Charlie Turner]

Created attachment 346706 [details]
Patch

Change some cosmetics from previous review, add files to Xcode project. It was not clear how Xcode calculates unique file identifiers, I just took the first 24-bits of the md5sum for the respective files

[Xabier Rodríguez Calvar]

Comment on attachment 346706 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=346706&action=review

> Source/WebCore/platform/graphics/iso/ISOProtectionSystemSpecificHeaderBox.cpp:2
> + * Copyright (C) 2018 Apple Inc. All rights reserved.

You're free, if you wish, to leave Apple's copyright here but you need to add your affiliation here as well as you (and your affiliation) are responsible of this code.

> Source/WebCore/platform/graphics/iso/ISOProtectionSystemSpecificHeaderBox.h:2
> + * Copyright (C) 2018 Apple Inc. All rights reserved.

Ditto.

[Charlie Turner]

Created attachment 346762 [details]
Patch

Add more copyright headers

[WebKit Commit Bot]

Comment on attachment 346762 [details]
Patch

Clearing flags on attachment: 346762

Committed r234689: <https://trac.webkit.org/changeset/234689>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/43041072>