From acbd84f5741451d67e0fbaa3b85fdafc85dab5f9 Mon Sep 17 00:00:00 2001 From: wolfbeast Date: Sun, 28 Jan 2018 10:25:49 +0100 Subject: Check for integer overflow in AesTask::DoCrypto() (DiD) After calling mResult.SetLength(mData.Length() + 16) we should check that the integer addition didn't overflow. It seems at the moment impossible to create ArrayBuffers of size >= 0x0xfffffff0, however adding a check here doesn't hurt. mResult.Length() is passed to the PK11 API functions as a maxOut parameter and should be checked by the softoken crypto algorithm implementations. AES-ECB and AES-GCM seem to do that correctly. --- dom/crypto/WebCryptoTask.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/dom/crypto/WebCryptoTask.cpp b/dom/crypto/WebCryptoTask.cpp index 57a7da186..f5fc7b5bc 100644 --- a/dom/crypto/WebCryptoTask.cpp +++ b/dom/crypto/WebCryptoTask.cpp @@ -716,6 +716,11 @@ private: return NS_ERROR_DOM_INVALID_ACCESS_ERR; } + // Check whether the integer addition would overflow. + if (std::numeric_limits::max() - 16 < mData.Length()) { + return NS_ERROR_DOM_DATA_ERR; + } + // Initialize the output buffer (enough space for padding / a full tag) uint32_t dataLen = mData.Length(); uint32_t maxLen = dataLen + 16; -- cgit v1.2.3 From c247ba5ab8f600fd748bc914524ae1ee17369062 Mon Sep 17 00:00:00 2001 From: wolfbeast Date: Fri, 6 Oct 2017 19:47:11 +0200 Subject: Stop bypassing the Xray layer when walking the prototype chain. --- js/xpconnect/src/XPCJSID.cpp | 29 ++++++++++++++--------------- 1 file changed, 14 insertions(+), 15 deletions(-) diff --git a/js/xpconnect/src/XPCJSID.cpp b/js/xpconnect/src/XPCJSID.cpp index b9cbee7be..1e14c1bdf 100644 --- a/js/xpconnect/src/XPCJSID.cpp +++ b/js/xpconnect/src/XPCJSID.cpp @@ -456,27 +456,26 @@ nsJSIID::Enumerate(nsIXPConnectWrappedNative* wrapper, static nsresult FindObjectForHasInstance(JSContext* cx, HandleObject objArg, MutableHandleObject target) { + using namespace mozilla::jsipc; RootedObject obj(cx, objArg), proto(cx); - - while (obj && !IS_WN_REFLECTOR(obj) && - !IsDOMObject(obj) && !mozilla::jsipc::IsCPOW(obj)) - { - if (js::IsWrapper(obj)) { - obj = js::CheckedUnwrap(obj, /* stopAtWindowProxy = */ false); - continue; + while (true) { + // Try the object, or the wrappee if allowed. + JSObject* o = js::IsWrapper(obj) ? js::CheckedUnwrap(obj, false) : obj; + if (o && (IS_WN_REFLECTOR(o) || IsDOMObject(o) || IsCPOW(o))) { + target.set(o); + return NS_OK; } - { - JSAutoCompartment ac(cx, obj); - if (!js::GetObjectProto(cx, obj, &proto)) - return NS_ERROR_FAILURE; + // Walk the prototype chain from the perspective of the callee (i.e. + // respecting Xrays if they exist). + if (!js::GetObjectProto(cx, obj, &proto)) + return NS_ERROR_FAILURE; + if (!proto) { + target.set(nullptr); + return NS_OK; } - obj = proto; } - - target.set(obj); - return NS_OK; } nsresult -- cgit v1.2.3 From b9b545e7ddcbbe5934d905805db1d6a436862737 Mon Sep 17 00:00:00 2001 From: wolfbeast Date: Wed, 4 Oct 2017 14:14:24 +0200 Subject: Avoid potentially unsafe snprintf usage in FPSCounter. snprintf returns the number of bytes it would have written when it runs out of space. This patch makes sure we properly handle this unlikely event in FPSCounter. This patch also makes sure we don't print out the contents of an uninitialized buffer. --- gfx/layers/composite/FPSCounter.cpp | 27 +++++++++++++++++++++------ 1 file changed, 21 insertions(+), 6 deletions(-) diff --git a/gfx/layers/composite/FPSCounter.cpp b/gfx/layers/composite/FPSCounter.cpp index 02ffc4b2c..b8e93eb97 100644 --- a/gfx/layers/composite/FPSCounter.cpp +++ b/gfx/layers/composite/FPSCounter.cpp @@ -210,7 +210,10 @@ FPSCounter::WriteFrameTimeStamps(PRFileDesc* fd) const int bufferSize = 256; char buffer[bufferSize]; int writtenCount = SprintfLiteral(buffer, "FPS Data for: %s\n", mFPSName); - MOZ_ASSERT(writtenCount >= 0); + MOZ_ASSERT(writtenCount < bufferSize); + if (writtenCount >= bufferSize) { + return; + } PR_Write(fd, buffer, writtenCount); ResetReverseIterator(); @@ -225,8 +228,10 @@ FPSCounter::WriteFrameTimeStamps(PRFileDesc* fd) while (HasNext(startTimeStamp)) { TimeDuration duration = previousSample - nextTimeStamp; writtenCount = SprintfLiteral(buffer, "%f,\n", duration.ToMilliseconds()); - - MOZ_ASSERT(writtenCount >= 0); + MOZ_ASSERT(writtenCount < bufferSize); + if (writtenCount >= bufferSize) { + continue; + } PR_Write(fd, buffer, writtenCount); previousSample = nextTimeStamp; @@ -299,8 +304,13 @@ FPSCounter::PrintFPS() void FPSCounter::PrintHistogram(std::map& aHistogram) { + if (aHistogram.size() == 0) { + return; + } + int length = 0; const int kBufferLength = 512; + int availableSpace = kBufferLength; char buffer[kBufferLength]; for (std::map::iterator iter = aHistogram.begin(); @@ -309,9 +319,14 @@ FPSCounter::PrintHistogram(std::map& aHistogram) int fps = iter->first; int count = iter->second; - length += snprintf(buffer + length, kBufferLength - length, - "FPS: %d = %d. ", fps, count); - NS_ASSERTION(length >= kBufferLength, "Buffer overrun while printing FPS histogram."); + int lengthRequired = snprintf(buffer + length, availableSpace, + "FPS: %d = %d. ", fps, count); + // Ran out of buffer space. Oh well - just print what we have. + if (lengthRequired > availableSpace) { + break; + } + length += lengthRequired; + availableSpace -= lengthRequired; } printf_stderr("%s\n", buffer); -- cgit v1.2.3 From 4099ff7494f2add95d35eb4ae0de12ab1fcf2aa2 Mon Sep 17 00:00:00 2001 From: wolfbeast Date: Fri, 6 Oct 2017 13:23:44 +0200 Subject: Fix ReadCompressedIndexDataValuesFromBlob(). --- dom/indexedDB/ActorsParent.cpp | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/dom/indexedDB/ActorsParent.cpp b/dom/indexedDB/ActorsParent.cpp index 702d5c985..c0cb69149 100644 --- a/dom/indexedDB/ActorsParent.cpp +++ b/dom/indexedDB/ActorsParent.cpp @@ -7,6 +7,7 @@ #include "ActorsParent.h" #include +#include // UINTPTR_MAX, uintptr_t #include "FileInfo.h" #include "FileManager.h" #include "IDBObjectStore.h" @@ -859,6 +860,11 @@ ReadCompressedIndexDataValuesFromBlob(const uint8_t* aBlobData, "ReadCompressedIndexDataValuesFromBlob", js::ProfileEntry::Category::STORAGE); + if (uintptr_t(aBlobData) > UINTPTR_MAX - aBlobDataLength) { + IDB_REPORT_INTERNAL_ERR(); + return NS_ERROR_FILE_CORRUPTED; + } + const uint8_t* blobDataIter = aBlobData; const uint8_t* blobDataEnd = aBlobData + aBlobDataLength; @@ -878,7 +884,8 @@ ReadCompressedIndexDataValuesFromBlob(const uint8_t* aBlobData, if (NS_WARN_IF(blobDataIter == blobDataEnd) || NS_WARN_IF(keyBufferLength > uint64_t(UINT32_MAX)) || - NS_WARN_IF(blobDataIter + keyBufferLength > blobDataEnd)) { + NS_WARN_IF(keyBufferLength > uintptr_t(blobDataEnd)) || + NS_WARN_IF(blobDataIter > blobDataEnd - keyBufferLength)) { IDB_REPORT_INTERNAL_ERR(); return NS_ERROR_FILE_CORRUPTED; } @@ -896,7 +903,8 @@ ReadCompressedIndexDataValuesFromBlob(const uint8_t* aBlobData, if (sortKeyBufferLength > 0) { if (NS_WARN_IF(blobDataIter == blobDataEnd) || NS_WARN_IF(sortKeyBufferLength > uint64_t(UINT32_MAX)) || - NS_WARN_IF(blobDataIter + sortKeyBufferLength > blobDataEnd)) { + NS_WARN_IF(sortKeyBufferLength > uintptr_t(blobDataEnd)) || + NS_WARN_IF(blobDataIter > blobDataEnd - sortKeyBufferLength)) { IDB_REPORT_INTERNAL_ERR(); return NS_ERROR_FILE_CORRUPTED; } -- cgit v1.2.3