AuthenticatorResponseData::decode should check bufferIsLargeEnoughToContain before allocating buffers This is a sub-task of Bug 209131. Bug 209131 – Don't allocate a buffer with the decoded size without ensuring bufferIsLargeEnoughToContain(size)
Created attachment 393635 [details] Patch
Created attachment 393637 [details] Patch
Comment on attachment 393637 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=393637&action=review Please fix all build failures. > Source/WebCore/Modules/webauthn/AuthenticatorResponseData.h:114 > + if (!decoder.template bufferIsLargeEnoughToContain<uint8_t>(rawIdLength)) This doesn't seem to be a valid C++ expression. > Source/WebCore/Modules/webauthn/AuthenticatorResponseData.h:134 > + if (!decoder.template bufferIsLargeEnoughToContain<uint8_t>(attestationObjectLength)) This doesn't seem to be a valid C++ expression. > Source/WebCore/Modules/webauthn/AuthenticatorResponseData.h:150 > + if (!decoder.template bufferIsLargeEnoughToContain<uint8_t>(authenticatorDataLength)) This doesn't seem to be a valid C++ expression. > Source/WebCore/Modules/webauthn/AuthenticatorResponseData.h:163 > + if (!decoder.template bufferIsLargeEnoughToContain<uint8_t>(signatureLength)) This doesn't seem to be a valid C++ expression. > Source/WebCore/Modules/webauthn/AuthenticatorResponseData.h:189 > + if (!decoder.template bufferIsLargeEnoughToContain<uint8_t>(userHandleLength)) This doesn't seem to be a valid C++ expression.
Thanks. Will fix.
Comment on attachment 393637 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=393637&action=review >> Source/WebCore/Modules/webauthn/AuthenticatorResponseData.h:114 >> + if (!decoder.template bufferIsLargeEnoughToContain<uint8_t>(rawIdLength)) > > This doesn't seem to be a valid C++ expression. Needs to be rawIdLength.value(), but otherwise looks valid to me. Are you just saying you’re not familiar with the ".template" syntax?
Oh, that part. See https://stackoverflow.com/q/3786360 https://stackoverflow.com/a/613132 Too long, I didn't read neither understand :-)
Created attachment 393682 [details] Patch
(In reply to Darin Adler from comment #5) > Comment on attachment 393637 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=393637&action=review > > >> Source/WebCore/Modules/webauthn/AuthenticatorResponseData.h:114 > >> + if (!decoder.template bufferIsLargeEnoughToContain<uint8_t>(rawIdLength)) > > > > This doesn't seem to be a valid C++ expression. > > Needs to be rawIdLength.value(), but otherwise looks valid to me. Are you > just saying you’re not familiar with the ".template" syntax? My bad, it has been a while since last time I have to use .template.
Comment on attachment 393682 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=393682&action=review > Source/WebCore/Modules/webauthn/AuthenticatorResponseData.h:120 > if (!rawIdLength) > return WTF::nullopt; > > - result.rawId = ArrayBuffer::create(rawIdLength.value(), sizeof(uint8_t)); > + if (!decoder.template bufferIsLargeEnoughToContain<uint8_t>(rawIdLength.value())) > + return WTF::nullopt; > + result.rawId = ArrayBuffer::tryCreate(rawIdLength.value(), sizeof(uint8_t)); > + if (!result.rawId) > + return WTF::nullopt; > if (!decoder.decodeFixedLengthData(reinterpret_cast<uint8_t*>(result.rawId->data()), rawIdLength.value(), 1)) > return WTF::nullopt; Feels to me like we should refactor so we can share more code. This pattern over and over again.
Comment on attachment 393682 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=393682&action=review >> Source/WebCore/Modules/webauthn/AuthenticatorResponseData.h:120 >> return WTF::nullopt; > > Feels to me like we should refactor so we can share more code. This pattern over and over again. Good idea. Will fix.
Created attachment 393904 [details] Patch
Created attachment 393929 [details] Patch
Comment on attachment 393929 [details] Patch Clearing flags on attachment: 393929 Committed r258676: <https://trac.webkit.org/changeset/258676>
All reviewed patches have been landed. Closing bug.
<rdar://problem/60615555>