decodeCFData should check bufferIsLargeEnoughToContain before allocating a buffer 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 393943 [details] Patch
Comment on attachment 393943 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=393943&action=review > Source/WebCore/platform/network/cf/CertificateInfo.h:118 > + if (!decoder.bufferIsLargeEnoughToContain<uint8_t>(static_cast<size_t>(size))) This cast to size_t doesn’t look right, nor does the code below. If the size is larger than what will fit in size_t, this will do the wrong thing. We can clean up this mix of types. CFDataCreate takes an argument of a type called CFIndex. So I think we need to write this: if (size > std::numeric_limits<CFIndex>::max()) return false; CFIndex length = size; Then we should use length instead of static_cast<size_t>(size) in both places that it appears.
I’m also thinking that we could use a helper to decode a size, check the size, allocate a vector of that size, then decode the data into the vector. I think this pattern is happening over and over again. And every time we are writing the same code.
(In reply to Darin Adler from comment #3) > I’m also thinking that we could use a helper to decode a size, check the > size, allocate a vector of that size, then decode the data into the vector. > I think this pattern is happening over and over again. And every time we are > writing the same code. Oops. There are already coder classes for Vector there. Will rewrite code with them. https://trac.webkit.org/browser/webkit/trunk/Source/WTF/wtf/persistence/PersistentCoders.h#L167 https://trac.webkit.org/browser/webkit/trunk/Source/WebKit/Platform/IPC/ArgumentCoders.h#L375
Great! Looking forward to seeing that version.
VectorArgumentCoder is just only for WTF::Vector. VectorArgumentCoder can't be used for this case of encodeCFData because encodeCFData doesn't treat WTF::Vector. I will use IPC::VectorArgumentCoder and WTF::Persistence::VectorCoder for other cases in ohter bug tickets.
encodeCFData doesn't need to convert CFIndex to uint64_t. If I understand correctly, CertificateInfo is passed only via IPC between web process and UI process, not in persistent storage. I think it's safe to change the encoding format.
WTF::Persistence::Encoder can't encode 'long'. WTF::Persistence::Encoder has int32_t (int) and int64_t (long long) encoders, but long. Even though both long and long long are 64bit, they are different types. > CompileC /Users/fujii/webkit/ga/WebKitBuild/WebCore.build/Debug/WebCore.build/Objects-normal/x86_64/ResourceResponseCocoa.o platform/network/cocoa/ResourceResponseCocoa.mm normal x86_64 objective-c++ com.apple.compilers.llvm.clang.1_0.compiler > cd /Users/fujii/webkit/ga/Source/WebCore > export LANG=en_US.US-ASCII > /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/clang -x objective-c++ -target x86_64-apple-macos10.15 -fmessage-length=0 -fdiagnostics-show-note-include-stack -fmacro-backtrace-limit=0 -std=gnu++1z -stdlib=libc++ -f$ > In file included from /Users/fujii/webkit/ga/Source/WebCore/platform/network/cocoa/ResourceResponseCocoa.mm:27: > In file included from /Users/fujii/webkit/ga/Source/WebCore/platform/network/cf/ResourceResponse.h:28: > In file included from /Users/fujii/webkit/ga/Source/WebCore/platform/network/ResourceResponseBase.h:30: > In file included from /Users/fujii/webkit/ga/Source/WebCore/platform/network/cf/CertificateInfo.h:32: > In file included from /Users/fujii/webkit/ga/WebKitBuild/Debug/usr/local/include/wtf/persistence/PersistentCoders.h:36: > In file included from /Users/fujii/webkit/ga/WebKitBuild/Debug/usr/local/include/wtf/persistence/PersistentDecoder.h:30: > /Users/fujii/webkit/ga/WebKitBuild/Debug/usr/local/include/wtf/persistence/PersistentCoder.h:37:10: error: member reference base type 'const long' is not a structure or union > t.encode(encoder); > ~^~~~~~~ > In file included from /Users/fujii/webkit/ga/Source/WebCore/platform/network/cocoa/ResourceResponseCocoa.mm:27: > In file included from /Users/fujii/webkit/ga/Source/WebCore/platform/network/cf/ResourceResponse.h:28: > In file included from /Users/fujii/webkit/ga/Source/WebCore/platform/network/ResourceResponseBase.h:30: > In file included from /Users/fujii/webkit/ga/Source/WebCore/platform/network/cf/CertificateInfo.h:32: > In file included from /Users/fujii/webkit/ga/WebKitBuild/Debug/usr/local/include/wtf/persistence/PersistentCoders.h:37: > /Users/fujii/webkit/ga/WebKitBuild/Debug/usr/local/include/wtf/persistence/PersistentEncoder.h:65:19: note: in instantiation of member function 'WTF::Persistence::Coder<long>::encode' requested here > Coder<T>::encode(*this, t); > ^ > /Users/fujii/webkit/ga/WebKitBuild/Debug/usr/local/include/wtf/persistence/PersistentEncoder.h:70:9: note: in instantiation of function template specialization 'WTF::Persistence::Encoder::encode<long>' requested here > encode(t); > ^ > In file included from /Users/fujii/webkit/ga/Source/WebCore/platform/network/cocoa/ResourceResponseCocoa.mm:27: > In file included from /Users/fujii/webkit/ga/Source/WebCore/platform/network/cf/ResourceResponse.h:28: > In file included from /Users/fujii/webkit/ga/Source/WebCore/platform/network/ResourceResponseBase.h:30: > /Users/fujii/webkit/ga/Source/WebCore/platform/network/cf/CertificateInfo.h:108:13: note: in instantiation of function template specialization 'WTF::Persistence::Encoder::operator<<<long>' requested here > encoder << length; > ^ > In file included from /Users/fujii/webkit/ga/Source/WebCore/platform/network/cocoa/ResourceResponseCocoa.mm:27: > In file included from /Users/fujii/webkit/ga/Source/WebCore/platform/network/cf/ResourceResponse.h:28: > In file included from /Users/fujii/webkit/ga/Source/WebCore/platform/network/ResourceResponseBase.h:30: > In file included from /Users/fujii/webkit/ga/Source/WebCore/platform/network/cf/CertificateInfo.h:32: > In file included from /Users/fujii/webkit/ga/WebKitBuild/Debug/usr/local/include/wtf/persistence/PersistentCoders.h:36: > In file included from /Users/fujii/webkit/ga/WebKitBuild/Debug/usr/local/include/wtf/persistence/PersistentDecoder.h:30: > /Users/fujii/webkit/ga/WebKitBuild/Debug/usr/local/include/wtf/persistence/PersistentCoder.h:42:16: error: type 'long' cannot be used prior to '::' because it has no members > return T::decode(decoder, t); > ^ > In file included from /Users/fujii/webkit/ga/Source/WebCore/platform/network/cocoa/ResourceResponseCocoa.mm:27: > In file included from /Users/fujii/webkit/ga/Source/WebCore/platform/network/cf/ResourceResponse.h:28: > In file included from /Users/fujii/webkit/ga/Source/WebCore/platform/network/ResourceResponseBase.h:30: > In file included from /Users/fujii/webkit/ga/Source/WebCore/platform/network/cf/CertificateInfo.h:32: > In file included from /Users/fujii/webkit/ga/WebKitBuild/Debug/usr/local/include/wtf/persistence/PersistentCoders.h:36: > /Users/fujii/webkit/ga/WebKitBuild/Debug/usr/local/include/wtf/persistence/PersistentDecoder.h:85:26: note: in instantiation of member function 'WTF::Persistence::Coder<long>::decode' requested here > return Coder<T>::decode(*this, t); > ^ > In file included from /Users/fujii/webkit/ga/Source/WebCore/platform/network/cocoa/ResourceResponseCocoa.mm:27: > In file included from /Users/fujii/webkit/ga/Source/WebCore/platform/network/cf/ResourceResponse.h:28: > In file included from /Users/fujii/webkit/ga/Source/WebCore/platform/network/ResourceResponseBase.h:30: > /Users/fujii/webkit/ga/Source/WebCore/platform/network/cf/CertificateInfo.h:115:18: note: in instantiation of function template specialization 'WTF::Persistence::Decoder::decode<long>' requested here > if (!decoder.decode(size)) > ^ > 2 errors generated.
I’m fixing this in bug 224984. *** This bug has been marked as a duplicate of bug 224984 ***