From e66b6818815c0ceff11374e5fa78f73a138b8948 Mon Sep 17 00:00:00 2001 From: chenkeyu Date: Fri, 22 Mar 2024 09:42:59 +0800 Subject: [PATCH] Fix wrong data padding in parcel Issue:https://gitee.com/openharmony/commonlibrary_c_utils/issues/I9AKE2?from=project-issue Signed-off-by: chenkeyu --- base/include/parcel.h | 2 + base/src/parcel.cpp | 50 ++++- .../fuzztest/parcel_fuzzer/parcel_fuzzer.cpp | 14 ++ .../unittest/common/utils_parcel_test.cpp | 191 +++++++++++++++++- 4 files changed, 247 insertions(+), 10 deletions(-) diff --git a/base/include/parcel.h b/base/include/parcel.h index 64c73f3..d715475 100644 --- a/base/include/parcel.h +++ b/base/include/parcel.h @@ -743,6 +743,8 @@ public: */ template bool ReadVector(std::vector *val, bool (Parcel::*Read)(T &)); + template + bool ReadFixedAlignVector(std::vector *val, bool (Parcel::*SpecialRead)(T2 &)); bool ReadBoolVector(std::vector *val); bool ReadInt8Vector(std::vector *val); bool ReadInt16Vector(std::vector *val); diff --git a/base/src/parcel.cpp b/base/src/parcel.cpp index 14b3b6b..4d8311d 100644 --- a/base/src/parcel.cpp +++ b/base/src/parcel.cpp @@ -531,6 +531,9 @@ bool Parcel::WriteCString(const char *value) return false; } int32_t dataLength = strlen(value); + if (dataLength < 0 || dataLength >= INT32_MAX) { + return false; + } int32_t desireCapacity = (dataLength + 1) * sizeof(char); return WriteBuffer(value, desireCapacity); } @@ -542,6 +545,9 @@ bool Parcel::WriteString(const std::string &value) } int32_t dataLength = value.length(); + if (dataLength < 0 || dataLength >= INT32_MAX) { + return false; + } int32_t typeSize = sizeof(char); int32_t desireCapacity = dataLength + typeSize; @@ -560,6 +566,9 @@ bool Parcel::WriteString16(const std::u16string &value) int32_t dataLength = value.length(); int32_t typeSize = sizeof(char16_t); + if (dataLength < 0 || dataLength > ((static_cast(INT32_MAX)) / typeSize - 1)) { + return false; + } int32_t desireCapacity = (dataLength + 1) * typeSize; if (!Write(dataLength)) { @@ -577,6 +586,9 @@ bool Parcel::WriteString16WithLength(const char16_t *value, size_t len) int32_t dataLength = len; int32_t typeSize = sizeof(char16_t); + if (dataLength < 0 || dataLength > ((static_cast(INT32_MAX)) / typeSize - 1)) { + return false; + } int32_t desireCapacity = (dataLength + 1) * typeSize; std::u16string u16str(reinterpret_cast(value), len); @@ -594,6 +606,9 @@ bool Parcel::WriteString8WithLength(const char *value, size_t len) } int32_t dataLength = len; + if (dataLength < 0 || dataLength >= INT32_MAX) { + return false; + } int32_t typeSize = sizeof(char); int32_t desireCapacity = (dataLength + 1) * typeSize; @@ -1276,6 +1291,9 @@ bool Parcel::WriteVector(const std::vector &val, bool (Parcel::*Write)(T2)) } size_t padSize = this->GetPadSize(val.size() * sizeof(T1)); + if (!EnsureWritableCapacity(padSize)) { + return false; + } this->WritePadBytes(padSize); return true; } @@ -1299,13 +1317,16 @@ bool Parcel::WriteFixedAlignVector(const std::vector &originVal, bool (Parce // The write length of these interfaces is different from the original type. // They need to use the specified write length and calculate the padSize based on this. size_t padSize = this->GetPadSize(originVal.size() * sizeof(Type)); + if (!EnsureWritableCapacity(padSize)) { + return false; + } this->WritePadBytes(padSize); return true; } bool Parcel::WriteBoolVector(const std::vector &val) { - return WriteVector(val, &Parcel::WriteBool); + return WriteFixedAlignVector(val, &Parcel::WriteBool); } bool Parcel::WriteInt8Vector(const std::vector &val) @@ -1315,7 +1336,7 @@ bool Parcel::WriteInt8Vector(const std::vector &val) bool Parcel::WriteInt16Vector(const std::vector &val) { - return WriteVector(val, &Parcel::WriteInt16); + return WriteFixedAlignVector(val, &Parcel::WriteInt16); } bool Parcel::WriteInt32Vector(const std::vector &val) @@ -1402,7 +1423,8 @@ bool Parcel::ReadVector(std::vector *val, bool (Parcel::*Read)(T &)) return true; } -bool Parcel::ReadBoolVector(std::vector *val) +template +bool Parcel::ReadFixedAlignVector(std::vector *val, bool (Parcel::*SpecialRead)(T2 &)) { if (val == nullptr) { return false; @@ -1413,10 +1435,11 @@ bool Parcel::ReadBoolVector(std::vector *val) return false; } - size_t readAbleSize = this->GetReadableBytes(); + size_t readAbleSize = this->GetReadableBytes() / sizeof(Type); size_t size = static_cast(len); - if ((size > readAbleSize) || (val->max_size() < size)) { - UTILS_LOGE("Failed to read bool vector, size = %{public}zu, readAbleSize = %{public}zu", size, readAbleSize); + if ((size > readAbleSize) || (size > val->max_size())) { + UTILS_LOGE("Failed to fixed aligned read vector, size = %{public}zu, readAbleSize = %{public}zu", + size, readAbleSize); return false; } val->resize(size); @@ -1425,14 +1448,23 @@ bool Parcel::ReadBoolVector(std::vector *val) } for (size_t i = 0; i < size; ++i) { - (*val)[i] = ReadBool(); + T2 parcelVal; + if (!(this->*SpecialRead)(parcelVal)) { + return false; + } + (*val)[i] = parcelVal; } - size_t padSize = this->GetPadSize(size * sizeof(bool)); + size_t padSize = this->GetPadSize(size * sizeof(Type)); this->SkipBytes(padSize); return true; } +bool Parcel::ReadBoolVector(std::vector *val) +{ + return ReadFixedAlignVector(val, &Parcel::ReadBool); +} + bool Parcel::ReadInt8Vector(std::vector *val) { return ReadVector(val, &Parcel::ReadInt8Unaligned); @@ -1440,7 +1472,7 @@ bool Parcel::ReadInt8Vector(std::vector *val) bool Parcel::ReadInt16Vector(std::vector *val) { - return ReadVector(val, &Parcel::ReadInt16); + return ReadFixedAlignVector(val, &Parcel::ReadInt16); } bool Parcel::ReadInt32Vector(std::vector *val) diff --git a/base/test/fuzztest/parcel_fuzzer/parcel_fuzzer.cpp b/base/test/fuzztest/parcel_fuzzer/parcel_fuzzer.cpp index bd3322d..7bfccfe 100644 --- a/base/test/fuzztest/parcel_fuzzer/parcel_fuzzer.cpp +++ b/base/test/fuzztest/parcel_fuzzer/parcel_fuzzer.cpp @@ -452,6 +452,20 @@ const std::vector> unaligned_o size_t bufferSize = dataProvider->ConsumeIntegralInRange(1, MAX_BUFFER_SIZE); parcel.RewindWrite(bufferSize); }, + [](FuzzedDataProvider* dataProvider, Parcel& parcel) { + FUZZ_LOGI("WriteBoolVector"); + size_t vectorSize = dataProvider->ConsumeIntegralInRange(1, MAX_VECTOR_SIZE); + std::vector data = dataProvider->ConsumeBytes(vectorSize); + if (data.size() > 0) { + std::vector testdata(data.size()); + for (size_t i = 0; i < testdata.size(); i++) { + testdata[i] = 1 & data[i]; + } + parcel.WriteBoolVector(testdata); + } + }, + + PARCEL_WRITE_VECTOR_WITH_BOOL_RETURN(int16_t, WriteInt16Vector), [](FuzzedDataProvider* dataProvider, Parcel& parcel) { FUZZ_LOGI("WriteBoolUnaligned"); diff --git a/base/test/unittest/common/utils_parcel_test.cpp b/base/test/unittest/common/utils_parcel_test.cpp index a736a33..07f841d 100644 --- a/base/test/unittest/common/utils_parcel_test.cpp +++ b/base/test/unittest/common/utils_parcel_test.cpp @@ -2011,5 +2011,194 @@ HWTEST_F(UtilsParcelTest, test_RewindWrite_003, TestSize.Level0) EXPECT_EQ(int32Read[3], 4); EXPECT_EQ(int32Read[4], 0); } + +HWTEST_F(UtilsParcelTest, test_VectorDataPadding_001, TestSize.Level0) +{ + Parcel parcel1(nullptr); + std::vector val1(121, true); + bool result = parcel1.WriteBoolVector(val1); + EXPECT_EQ(result, true); + + int32_t targetVal = 123; + parcel1.WriteInt32(targetVal); + + std::vector val2; + result = parcel1.ReadBoolVector(&val2); + int32_t target = parcel1.ReadInt32(); + EXPECT_EQ(target, targetVal); +} + +HWTEST_F(UtilsParcelTest, test_VectorDataPadding_002, TestSize.Level0) +{ + Parcel parcel1(nullptr); + std::vector val1(15, true); + bool result = parcel1.WriteBoolVector(val1); + EXPECT_EQ(result, true); + + std::vector val2(16, true); + result = parcel1.WriteBoolVector(val2); + EXPECT_EQ(result, true); + + std::vector val3; + result = parcel1.ReadBoolVector(&val3); + for (int i = 0; i < val1.size(); i++) { + EXPECT_EQ(val1[i], val3[i]); + } + + std::vector val4; + result = parcel1.ReadBoolVector(&val4); + for (int i = 0; i < val2.size(); i++) { + EXPECT_EQ(val2[i], val4[i]); + } + parcel1.FlushBuffer(); + + result = parcel1.WriteBoolVector(val2); + EXPECT_EQ(result, true); + result = parcel1.WriteBoolVector(val1); + EXPECT_EQ(result, true); + + std::vector val5; + result = parcel1.ReadBoolVector(&val5); + for (int i = 0; i < val2.size(); i++) { + EXPECT_EQ(val2[i], val5[i]); + } + + std::vector val6; + result = parcel1.ReadBoolVector(&val6); + for (int i = 0; i < val1.size(); i++) { + EXPECT_EQ(val1[i], val6[i]); + } +} + +HWTEST_F(UtilsParcelTest, test_VectorDataPadding_003, TestSize.Level0) +{ + Parcel parcel1(nullptr); + std::vector val1(17, true); + bool result = parcel1.WriteBoolVector(val1); + EXPECT_EQ(result, true); + + std::vector val2(18, 1); + result = parcel1.WriteInt16Vector(val2); + EXPECT_EQ(result, true); + + std::vector val3; + result = parcel1.ReadBoolVector(&val3); + for (int i = 0; i < val1.size(); i++) { + EXPECT_EQ(val1[i], val3[i]); + } + + std::vector val4; + result = parcel1.ReadInt16Vector(&val4); + for (int i = 0; i < val2.size(); i++) { + EXPECT_EQ(val2[i], val4[i]); + } + parcel1.FlushBuffer(); + + result = parcel1.WriteInt16Vector(val2); + EXPECT_EQ(result, true); + result = parcel1.WriteBoolVector(val1); + EXPECT_EQ(result, true); + + std::vector val5; + result = parcel1.ReadInt16Vector(&val5); + for (int i = 0; i < val2.size(); i++) { + EXPECT_EQ(val2[i], val5[i]); + } + + std::vector val6; + result = parcel1.ReadBoolVector(&val6); + for (int i = 0; i < val1.size(); i++) { + EXPECT_EQ(val1[i], val6[i]); + } +} + +HWTEST_F(UtilsParcelTest, test_VectorDataPadding_004, TestSize.Level0) +{ + Parcel parcel1(nullptr); + std::vector val1(19, 1); + bool result = parcel1.WriteInt16Vector(val1); + EXPECT_EQ(result, true); + + std::vector val2(20, 1); + result = parcel1.WriteInt16Vector(val2); + EXPECT_EQ(result, true); + + std::vector val3; + result = parcel1.ReadInt16Vector(&val3); + for (int i = 0; i < val1.size(); i++) { + EXPECT_EQ(val1[i], val3[i]); + } + + std::vector val4; + result = parcel1.ReadInt16Vector(&val4); + for (int i = 0; i < val2.size(); i++) { + EXPECT_EQ(val2[i], val4[i]); + } + parcel1.FlushBuffer(); + + result = parcel1.WriteInt16Vector(val2); + EXPECT_EQ(result, true); + result = parcel1.WriteInt16Vector(val1); + EXPECT_EQ(result, true); + + std::vector val5; + result = parcel1.ReadInt16Vector(&val5); + for (int i = 0; i < val2.size(); i++) { + EXPECT_EQ(val2[i], val5[i]); + } + + std::vector val6; + result = parcel1.ReadInt16Vector(&val6); + for (int i = 0; i < val1.size(); i++) { + EXPECT_EQ(val1[i], val6[i]); + } +} + +HWTEST_F(UtilsParcelTest, test_WriteStringDataLength_001, TestSize.Level0) +{ + Parcel parcel1(nullptr); + + std::string veryLongString(static_cast(INT32_MAX) + 1, '#'); + bool result = parcel1.WriteCString(veryLongString.c_str()); + EXPECT_EQ(result, false); + parcel1.FlushBuffer(); + + result = parcel1.WriteString(veryLongString); + EXPECT_EQ(result, false); + parcel1.FlushBuffer(); + + std::u16string veryLongStringU16(static_cast(INT32_MAX) / 2, '#'); + result = parcel1.WriteString16(veryLongStringU16); + EXPECT_EQ(result, false); + parcel1.FlushBuffer(); + + result = parcel1.WriteString16WithLength(veryLongStringU16.c_str(), static_cast(INT32_MAX) / 2); + EXPECT_EQ(result, false); + parcel1.FlushBuffer(); + + result = parcel1.WriteString8WithLength(veryLongString.c_str(), static_cast(INT32_MAX) + 1); + EXPECT_EQ(result, false); + parcel1.FlushBuffer(); + + result = parcel1.WriteCString(veryLongString.substr(0, DEFAULT_CPACITY - 1).c_str()); + EXPECT_EQ(result, true); + parcel1.FlushBuffer(); + + result = parcel1.WriteString(veryLongString.substr(0, DEFAULT_CPACITY - 5)); + EXPECT_EQ(result, true); + parcel1.FlushBuffer(); + + result = parcel1.WriteString16(veryLongStringU16.substr(0, (DEFAULT_CPACITY - 4) / 2 - 1)); + EXPECT_EQ(result, true); + parcel1.FlushBuffer(); + + result = parcel1.WriteString16WithLength(veryLongStringU16.c_str(), (DEFAULT_CPACITY - 4) / 2 - 1); + EXPECT_EQ(result, true); + parcel1.FlushBuffer(); + + result = parcel1.WriteString8WithLength(veryLongString.c_str(), DEFAULT_CPACITY - 5); + EXPECT_EQ(result, true); + parcel1.FlushBuffer(); +} } // namespace -} // namespace OHOS \ No newline at end of file +} // namespace OHOS -- Gitee