From c71eb45240a64e5419ab12e25dee3f9e07429da4 Mon Sep 17 00:00:00 2001 From: Nik Bougalis Date: Wed, 7 Nov 2018 11:55:24 -0800 Subject: [PATCH] Eliminate potential undefined behavior (RIPD-1685): Under certain conditions, we could call `memcpy` or `memcmp` with a null source pointer. Even when specifying 0 as the amount of data to copy this could result in undefined behavior under the C and C++ standards. Acknowledgements: Ripple thanks Guido Vranken for responsibly disclosing these issues. Bug Bounties and Responsible Disclosures: We welcome reviews of the rippled code and urge researchers to responsibly disclose any issues that they may find. For more on Ripple's Bug Bounty program, please visit: https://ripple.com/bug-bounty --- src/ripple/basics/Buffer.h | 8 ++++++-- src/ripple/protocol/STBlob.h | 32 +------------------------------- src/ripple/protocol/Serializer.h | 12 ++++++------ 3 files changed, 13 insertions(+), 39 deletions(-) diff --git a/src/ripple/basics/Buffer.h b/src/ripple/basics/Buffer.h index f9af0d84d..d19afa661 100644 --- a/src/ripple/basics/Buffer.h +++ b/src/ripple/basics/Buffer.h @@ -221,9 +221,13 @@ public: inline bool operator==(Buffer const& lhs, Buffer const& rhs) noexcept { - if (lhs.size () != rhs.size ()) + if (lhs.size() != rhs.size()) return false; - return !std::memcmp (lhs.data (), rhs.data (), lhs.size ()); + + if (lhs.size() == 0) + return true; + + return std::memcmp(lhs.data(), rhs.data(), lhs.size()) == 0; } inline bool operator!=(Buffer const& lhs, Buffer const& rhs) noexcept diff --git a/src/ripple/protocol/STBlob.h b/src/ripple/protocol/STBlob.h index a34cfaab5..1c069bd92 100644 --- a/src/ripple/protocol/STBlob.h +++ b/src/ripple/protocol/STBlob.h @@ -23,6 +23,7 @@ #include #include #include +#include #include #include @@ -42,18 +43,6 @@ public: { } - /** Construct with size and initializer. - Init will be called as: - void(void* data, std::size_t size) - */ - template - STBlob (SField const& f, std::size_t size, - Init&& init) - : STBase(f), value_ (size) - { - init(value_.data(), value_.size()); - } - STBlob (SField const& f, void const* data, std::size_t size) : STBase(f), value_ (data, size) @@ -115,12 +104,6 @@ public: s.addVL (value_.data (), value_.size ()); } - Buffer const& - peekValue () const - { - return value_; - } - STBlob& operator= (Slice const& slice) { @@ -141,25 +124,12 @@ public: return *this; } - Buffer& - peekValue () - { - return value_; - } - void setValue (Buffer&& b) { value_ = std::move (b); } - void - setValue (void const* data, std::size_t size) - { - value_.alloc (size); - std::memcpy(value_.data(), data, size); - } - bool isEquivalent (const STBase& t) const override; diff --git a/src/ripple/protocol/Serializer.h b/src/ripple/protocol/Serializer.h index 094898415..e804f0880 100644 --- a/src/ripple/protocol/Serializer.h +++ b/src/ripple/protocol/Serializer.h @@ -49,14 +49,14 @@ public: mData.reserve (n); } - Serializer (void const* data, - std::size_t size) + Serializer (void const* data, std::size_t size) { + assert(!data == !size); + mData.resize(size); - std::memcpy(mData.data(), - reinterpret_cast< - unsigned char const*>( - data), size); + + if (size) + std::memcpy(mData.data(), data, size); } Slice slice() const noexcept