diff --git a/base/include/parcel.h b/base/include/parcel.h index d715475d5eb7dd1a117b3d73dd4ac926536f40c6..882fac69ad5b6e2b8b680343f2bbfd8aa365019e 100644 --- a/base/include/parcel.h +++ b/base/include/parcel.h @@ -513,6 +513,14 @@ public: */ const uint8_t *ReadBuffer(size_t length); + /** + * @brief Read a block of data (buffer data) from this parcel. + * + * @param length Size of the buffer(Bytes). + * @return A `uint8_t` pointer to the buffer. + */ + const uint8_t *ReadBuffer(size_t length, bool isValidate); + /** * @brief Reads a block of data (buffer data) without padding (alignment) * from this parcel. @@ -825,6 +833,8 @@ private: bool WriteParcelableOffset(size_t offset); const uint8_t *BasicReadBuffer(size_t length); + + bool IsReadObjectData(const size_t nextObj, const size_t upperBound); bool ValidateReadData(size_t upperBound); diff --git a/base/src/parcel.cpp b/base/src/parcel.cpp index ef607c541b8232449fcd87928b33d3939b9d808a..64fcfc8963d39aca2b28d346993d842a496252c3 100644 --- a/base/src/parcel.cpp +++ b/base/src/parcel.cpp @@ -21,6 +21,8 @@ namespace OHOS { static const size_t DEFAULT_CPACITY = 204800; // 200K static const size_t CAPACITY_THRESHOLD = 4096; // 4k +static const int BINDER_TYPE_HANDLE = 0x73682a85; // binder header type handle +static const int BINDER_TYPE_FD = 0x66642a85; // binder header type fd Parcelable::Parcelable() : Parcelable(false) {} @@ -150,6 +152,18 @@ bool Parcel::EnsureWritableCapacity(size_t desireCapacity) return false; } +bool Parcel::IsReadObjectData(const size_t nextObj, const size_t upperBound) +{ + binder_size_t *objects = objectOffsets_; + auto offset = objects[nextObj]; + auto currentObject = reinterpret_cast(data_ + offset); + if (currentObject->hdr.type == BINDER_TYPE_FD || currentObject->hdr.type == BINDER_TYPE_HANDLE) { + return true; + } + UTILS_LOGE("Non-object Read object data, readPos = %{public}zu, upperBound = %{public}zu", readCursor_, upperBound); + return false; +} + // ValidateReadData only works in basic type read. It doesn't work when read remote object. // And read/write remote object has no effect on "nextObjectIdx_". bool Parcel::ValidateReadData([[maybe_unused]]size_t upperBound) @@ -165,9 +179,7 @@ bool Parcel::ValidateReadData([[maybe_unused]]size_t upperBound) size_t nextObj = nextObjectIdx_; do { if (readPos < objects[nextObj] + sizeof(parcel_flat_binder_object)) { - UTILS_LOGE("Non-object Read object data, readPos = %{public}zu, upperBound = %{public}zu", - readPos, upperBound); - return false; + return IsReadObjectData(nextObj, upperBound); } nextObj++; } while (nextObj < objSize && upperBound > objects[nextObj]); @@ -646,6 +658,23 @@ bool Parcel::EnsureObjectsCapacity() return true; } +const uint8_t *Parcel::ReadBuffer(size_t length, bool isValidate) +{ + if (GetReadableBytes() >= length) { + uint8_t *buffer = data_ + readCursor_; +#ifdef PARCEL_OBJECT_CHECK + size_t upperBound = readCursor_ + length; + if (isValidate && !ValidateReadData(upperBound)) { + return nullptr; + } +#endif + readCursor_ += length; + return buffer; + } + + return nullptr; +} + bool Parcel::WriteObjectOffset(binder_size_t offset) { if (offset > dataSize_) { @@ -787,6 +816,12 @@ const uint8_t *Parcel::ReadBuffer(size_t length) { if (GetReadableBytes() >= length) { uint8_t *buffer = data_ + readCursor_; +#ifdef PARCEL_OBJECT_CHECK + size_t upperBound = readCursor_ + length; + if (!ValidateReadData(upperBound)) { + return nullptr; + } +#endif readCursor_ += length; return buffer; } @@ -815,6 +850,12 @@ const uint8_t *Parcel::ReadUnpadBuffer(size_t length) { if (GetReadableBytes() >= length) { uint8_t *buffer = data_ + readCursor_; +#ifdef PARCEL_OBJECT_CHECK + size_t upperBound = readCursor_ + length; + if (!ValidateReadData(upperBound)) { + return nullptr; + } +#endif readCursor_ += length; SkipBytes(GetPadSize(length)); return buffer; diff --git a/base/test/benchmarktest/parcel_benchmark_test/parcel_benchmark_test.cpp b/base/test/benchmarktest/parcel_benchmark_test/parcel_benchmark_test.cpp index debd6d98b819337d6b50e93c8137cf7fd0f39b87..5665732b592e74e4f3beeea228d75de27f6d38f1 100644 --- a/base/test/benchmarktest/parcel_benchmark_test/parcel_benchmark_test.cpp +++ b/base/test/benchmarktest/parcel_benchmark_test/parcel_benchmark_test.cpp @@ -103,7 +103,7 @@ bool RemoteObject::Marshalling(Parcel &parcel) const sptr RemoteObject::Unmarshalling(Parcel &parcel) { BENCHMARK_LOGD("ParcelTest sptr RemoteObject::Unmarshalling(Parcel &parcel) is called."); - const uint8_t *buffer = parcel.ReadBuffer(sizeof(parcel_flat_binder_object)); + const uint8_t *buffer = parcel.ReadBuffer(sizeof(parcel_flat_binder_object), false); if (buffer == nullptr) { return nullptr; } diff --git a/base/test/fuzztest/parcel_fuzzer/parcel_fuzzer.cpp b/base/test/fuzztest/parcel_fuzzer/parcel_fuzzer.cpp index 7bfccfefff4eca0017c1810f630f7c3d97f6dd3c..b457f564082267ee1ab7294e736294c25ae9498b 100644 --- a/base/test/fuzztest/parcel_fuzzer/parcel_fuzzer.cpp +++ b/base/test/fuzztest/parcel_fuzzer/parcel_fuzzer.cpp @@ -145,7 +145,7 @@ bool RemoteObject::Marshalling(Parcel& parcel) const sptr RemoteObject::Unmarshalling(Parcel& parcel) { - const uint8_t* buffer = parcel.ReadBuffer(sizeof(parcel_flat_binder_object)); + const uint8_t* buffer = parcel.ReadBuffer(sizeof(parcel_flat_binder_object), false); if (buffer == nullptr) { return nullptr; } diff --git a/base/test/unittest/common/utils_parcel_test.cpp b/base/test/unittest/common/utils_parcel_test.cpp index 7cd836d512b68424d5fa555f81e7ef96cb444586..65556dd5d8e1b910060c7c275b1f90f13203ee75 100644 --- a/base/test/unittest/common/utils_parcel_test.cpp +++ b/base/test/unittest/common/utils_parcel_test.cpp @@ -66,7 +66,7 @@ bool RemoteObject::Marshalling(Parcel &parcel) const sptr RemoteObject::Unmarshalling(Parcel &parcel) { - const uint8_t *buffer = parcel.ReadBuffer(sizeof(parcel_flat_binder_object)); + const uint8_t *buffer = parcel.ReadBuffer(sizeof(parcel_flat_binder_object), false); if (buffer == nullptr) { return nullptr; }