From c135179c935e3f85e591014e14be81b3f2597825 Mon Sep 17 00:00:00 2001 From: Kevin May Date: Tue, 28 Jul 2020 11:29:04 +0100 Subject: IVGCVSW-5079 Fix for Timeline decoder segfaults when given bad data * Check packet size/length in ReadSwTraceMessage * Update existing Unit tests * Add new Unit tests Signed-off-by: Kevin May Change-Id: Ie15be8bc289d7bcb354a259312aada5268bff4e4 --- src/profiling/ProfilingUtils.cpp | 24 ++++++++++- src/profiling/ProfilingUtils.hpp | 2 +- src/profiling/test/BufferTests.cpp | 46 +++++++++++++++++++++- src/profiling/test/SendTimelinePacketTests.cpp | 12 +++--- .../TimelineDirectoryCaptureCommandHandler.cpp | 4 +- src/timelineDecoder/tests/TimelineTests.cpp | 4 +- .../profiling/gatordmock/tests/GatordMockTests.cpp | 6 ++- 7 files changed, 85 insertions(+), 13 deletions(-) diff --git a/src/profiling/ProfilingUtils.cpp b/src/profiling/ProfilingUtils.cpp index d86adbc051..8c43a8cd3a 100644 --- a/src/profiling/ProfilingUtils.cpp +++ b/src/profiling/ProfilingUtils.cpp @@ -316,7 +316,9 @@ uint32_t CalculateSizeOfPaddedSwString(const std::string& str) } // Read TimelineMessageDirectoryPacket from given IPacketBuffer and offset -SwTraceMessage ReadSwTraceMessage(const unsigned char* packetBuffer, unsigned int& offset) +SwTraceMessage ReadSwTraceMessage(const unsigned char* packetBuffer, + unsigned int& offset, + const unsigned int& packetLength) { ARMNN_ASSERT(packetBuffer); @@ -335,6 +337,11 @@ SwTraceMessage ReadSwTraceMessage(const unsigned char* packetBuffer, unsigned in offset += uint32_t_size; uint32_t swTraceDeclNameLength = ReadUint32(packetBuffer, offset); + if (swTraceDeclNameLength == 0 || swTraceDeclNameLength > packetLength) + { + throw RuntimeException("Error swTraceDeclNameLength is an invalid size", CHECK_LOCATION()); + } + offset += uint32_t_size; std::vector swTraceStringBuffer(swTraceDeclNameLength - 1); std::memcpy(swTraceStringBuffer.data(), @@ -346,6 +353,11 @@ SwTraceMessage ReadSwTraceMessage(const unsigned char* packetBuffer, unsigned in offset += CalculateSizeOfPaddedSwString(swTraceMessage.m_Name); uint32_t swTraceUINameLength = ReadUint32(packetBuffer, offset); + if (swTraceUINameLength == 0 || swTraceUINameLength > packetLength) + { + throw RuntimeException("Error swTraceUINameLength is an invalid size", CHECK_LOCATION()); + } + offset += uint32_t_size; swTraceStringBuffer.resize(swTraceUINameLength - 1); std::memcpy(swTraceStringBuffer.data(), @@ -357,6 +369,11 @@ SwTraceMessage ReadSwTraceMessage(const unsigned char* packetBuffer, unsigned in offset += CalculateSizeOfPaddedSwString(swTraceMessage.m_UiName); uint32_t swTraceArgTypesLength = ReadUint32(packetBuffer, offset); + if (swTraceArgTypesLength == 0 || swTraceArgTypesLength > packetLength) + { + throw RuntimeException("Error swTraceArgTypesLength is an invalid size", CHECK_LOCATION()); + } + offset += uint32_t_size; swTraceStringBuffer.resize(swTraceArgTypesLength - 1); std::memcpy(swTraceStringBuffer.data(), @@ -370,6 +387,11 @@ SwTraceMessage ReadSwTraceMessage(const unsigned char* packetBuffer, unsigned in offset += CalculateSizeOfPaddedSwString(swTraceString); uint32_t swTraceArgNamesLength = ReadUint32(packetBuffer, offset); + if (swTraceArgNamesLength == 0 || swTraceArgNamesLength > packetLength) + { + throw RuntimeException("Error swTraceArgNamesLength is an invalid size", CHECK_LOCATION()); + } + offset += uint32_t_size; swTraceStringBuffer.resize(swTraceArgNamesLength - 1); std::memcpy(swTraceStringBuffer.data(), diff --git a/src/profiling/ProfilingUtils.hpp b/src/profiling/ProfilingUtils.hpp index 985c49ef10..833b73d963 100644 --- a/src/profiling/ProfilingUtils.hpp +++ b/src/profiling/ProfilingUtils.hpp @@ -214,7 +214,7 @@ enum class TimelinePacketStatus uint32_t CalculateSizeOfPaddedSwString(const std::string& str); -SwTraceMessage ReadSwTraceMessage(const unsigned char*, unsigned int& offset); +SwTraceMessage ReadSwTraceMessage(const unsigned char*, unsigned int&, const unsigned int& packetLength); TimelinePacketStatus WriteTimelineLabelBinaryPacket(uint64_t profilingGuid, const std::string& label, diff --git a/src/profiling/test/BufferTests.cpp b/src/profiling/test/BufferTests.cpp index 0225d750a7..804335138d 100644 --- a/src/profiling/test/BufferTests.cpp +++ b/src/profiling/test/BufferTests.cpp @@ -1,5 +1,5 @@ // -// Copyright © 2019 Arm Ltd. All rights reserved. +// Copyright © 2019 Arm Ltd and Contributors. All rights reserved. // SPDX-License-Identifier: MIT // @@ -363,4 +363,48 @@ BOOST_AUTO_TEST_CASE(BufferMarkReadTest) BOOST_TEST(packetBuffer3.get()); } +BOOST_AUTO_TEST_CASE(ReadSwTraceMessageExceptionTest0) +{ + IPacketBufferPtr packetBuffer = std::make_unique(512); + + BOOST_TEST(packetBuffer->GetSize() == 0); + + // Write zero data to the buffer + WriteUint32(packetBuffer, 0, 0); + WriteUint32(packetBuffer, 4, 0); + WriteUint32(packetBuffer, 8, 0); + WriteUint32(packetBuffer, 12, 0); + + // Commit + packetBuffer->Commit(16); + + unsigned int uint32_t_size = sizeof(uint32_t); + unsigned int offset = uint32_t_size; + BOOST_CHECK_THROW(ReadSwTraceMessage(packetBuffer->GetReadableData(), offset, packetBuffer->GetSize()), + armnn::RuntimeException); + +} + +BOOST_AUTO_TEST_CASE(ReadSwTraceMessageExceptionTest1) +{ + IPacketBufferPtr packetBuffer = std::make_unique(512); + + BOOST_TEST(packetBuffer->GetSize() == 0); + + // Write data to the buffer + WriteUint32(packetBuffer, 0, 10); + WriteUint32(packetBuffer, 4, 20); + WriteUint32(packetBuffer, 8, 30); + WriteUint32(packetBuffer, 12, 40); + + // Commit + packetBuffer->Commit(16); + + unsigned int uint32_t_size = sizeof(uint32_t); + unsigned int offset = uint32_t_size; + BOOST_CHECK_THROW(ReadSwTraceMessage(packetBuffer->GetReadableData(), offset, packetBuffer->GetSize()), + armnn::RuntimeException); + +} + BOOST_AUTO_TEST_SUITE_END() diff --git a/src/profiling/test/SendTimelinePacketTests.cpp b/src/profiling/test/SendTimelinePacketTests.cpp index da30cef90c..5e9f6bddea 100644 --- a/src/profiling/test/SendTimelinePacketTests.cpp +++ b/src/profiling/test/SendTimelinePacketTests.cpp @@ -72,7 +72,9 @@ BOOST_AUTO_TEST_CASE(SendTimelineMessageDirectoryPackageTest) BOOST_CHECK(DeclCount == 5); offset += uint32_t_size; - SwTraceMessage swTraceMessage = ReadSwTraceMessage(packetBuffer->GetReadableData(), offset); + SwTraceMessage swTraceMessage = ReadSwTraceMessage(packetBuffer->GetReadableData(), + offset, + packetBuffer->GetSize()); BOOST_CHECK(swTraceMessage.m_Id == 0); BOOST_CHECK(swTraceMessage.m_Name == "declareLabel"); @@ -84,7 +86,7 @@ BOOST_AUTO_TEST_CASE(SendTimelineMessageDirectoryPackageTest) BOOST_CHECK(swTraceMessage.m_ArgNames[0] == "guid"); BOOST_CHECK(swTraceMessage.m_ArgNames[1] == "value"); - swTraceMessage = ReadSwTraceMessage(packetBuffer->GetReadableData(), offset); + swTraceMessage = ReadSwTraceMessage(packetBuffer->GetReadableData(), offset, packetBuffer->GetSize()); BOOST_CHECK(swTraceMessage.m_Id == 1); BOOST_CHECK(swTraceMessage.m_Name == "declareEntity"); @@ -94,7 +96,7 @@ BOOST_AUTO_TEST_CASE(SendTimelineMessageDirectoryPackageTest) BOOST_CHECK(swTraceMessage.m_ArgNames.size() == 1); BOOST_CHECK(swTraceMessage.m_ArgNames[0] == "guid"); - swTraceMessage = ReadSwTraceMessage(packetBuffer->GetReadableData(), offset); + swTraceMessage = ReadSwTraceMessage(packetBuffer->GetReadableData(), offset, packetBuffer->GetSize()); BOOST_CHECK(swTraceMessage.m_Id == 2); BOOST_CHECK(swTraceMessage.m_Name == "declareEventClass"); @@ -106,7 +108,7 @@ BOOST_AUTO_TEST_CASE(SendTimelineMessageDirectoryPackageTest) BOOST_CHECK(swTraceMessage.m_ArgNames[0] == "guid"); BOOST_CHECK(swTraceMessage.m_ArgNames[1] == "nameGuid"); - swTraceMessage = ReadSwTraceMessage(packetBuffer->GetReadableData(), offset); + swTraceMessage = ReadSwTraceMessage(packetBuffer->GetReadableData(), offset, packetBuffer->GetSize()); BOOST_CHECK(swTraceMessage.m_Id == 3); BOOST_CHECK(swTraceMessage.m_Name == "declareRelationship"); @@ -124,7 +126,7 @@ BOOST_AUTO_TEST_CASE(SendTimelineMessageDirectoryPackageTest) BOOST_CHECK(swTraceMessage.m_ArgNames[3] == "tailGuid"); BOOST_CHECK(swTraceMessage.m_ArgNames[4] == "attributeGuid"); - swTraceMessage = ReadSwTraceMessage(packetBuffer->GetReadableData(), offset); + swTraceMessage = ReadSwTraceMessage(packetBuffer->GetReadableData(), offset, packetBuffer->GetSize()); BOOST_CHECK(swTraceMessage.m_Id == 4); BOOST_CHECK(swTraceMessage.m_Name == "declareEvent"); diff --git a/src/timelineDecoder/TimelineDirectoryCaptureCommandHandler.cpp b/src/timelineDecoder/TimelineDirectoryCaptureCommandHandler.cpp index 74aefea142..5aac77c86a 100644 --- a/src/timelineDecoder/TimelineDirectoryCaptureCommandHandler.cpp +++ b/src/timelineDecoder/TimelineDirectoryCaptureCommandHandler.cpp @@ -1,5 +1,5 @@ // -// Copyright © 2019 Arm Ltd. All rights reserved. +// Copyright © 2019 Arm Ltd and Contributors. All rights reserved. // SPDX-License-Identifier: MIT // @@ -40,7 +40,7 @@ void TimelineDirectoryCaptureCommandHandler::ParseData(const armnn::profiling::P for (uint32_t declaration = 0; declaration < numberOfDeclarations; ++declaration) { - m_SwTraceMessages.push_back(profiling::ReadSwTraceMessage(data, offset)); + m_SwTraceMessages.push_back(profiling::ReadSwTraceMessage(data, offset, packet.GetLength())); } m_TimelineCaptureCommandHandler.SetThreadIdSize(m_SwTraceHeader.m_ThreadIdBytes); diff --git a/src/timelineDecoder/tests/TimelineTests.cpp b/src/timelineDecoder/tests/TimelineTests.cpp index 08d29d0f6a..ceb955d131 100644 --- a/src/timelineDecoder/tests/TimelineTests.cpp +++ b/src/timelineDecoder/tests/TimelineTests.cpp @@ -114,7 +114,9 @@ BOOST_AUTO_TEST_CASE(TimelineDirectoryTest) offset += uint32_t_size; for(uint32_t i = 0; i < declarationSize; ++i) { - swTraceBufferMessages.push_back(profiling::ReadSwTraceMessage(packetBuffer->GetReadableData(), offset)); + swTraceBufferMessages.push_back(profiling::ReadSwTraceMessage(packetBuffer->GetReadableData(), + offset, + packetBuffer->GetSize())); } SendTimelinePacketToCommandHandler(packetBuffer->GetReadableData(), timelineDirectoryCaptureCommandHandler); diff --git a/tests/profiling/gatordmock/tests/GatordMockTests.cpp b/tests/profiling/gatordmock/tests/GatordMockTests.cpp index cefb7231de..e7eb0846c0 100644 --- a/tests/profiling/gatordmock/tests/GatordMockTests.cpp +++ b/tests/profiling/gatordmock/tests/GatordMockTests.cpp @@ -1,5 +1,5 @@ // -// Copyright © 2017 Arm Ltd. All rights reserved. +// Copyright © 2017 Arm Ltd and Contributors. All rights reserved. // SPDX-License-Identifier: MIT // @@ -158,7 +158,9 @@ void CheckTimelineDirectory(timelinedecoder::TimelineDirectoryCaptureCommandHand offset += uint32_t_size; for(uint32_t i = 0; i < declarationSize; ++i) { - swTraceBufferMessages.push_back(profiling::ReadSwTraceMessage(packetBuffer->GetReadableData(), offset)); + swTraceBufferMessages.push_back(profiling::ReadSwTraceMessage(packetBuffer->GetReadableData(), + offset, + packetBuffer->GetSize())); } for(uint32_t index = 0; index < declarationSize; ++index) -- cgit v1.2.1