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 <kevin.may@arm.com>
Change-Id: Ie15be8bc289d7bcb354a259312aada5268bff4e4
diff --git a/src/profiling/ProfilingUtils.cpp b/src/profiling/ProfilingUtils.cpp
index d86adbc..8c43a8c 100644
--- a/src/profiling/ProfilingUtils.cpp
+++ b/src/profiling/ProfilingUtils.cpp
@@ -316,7 +316,9 @@
}
// 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 @@
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<unsigned char> swTraceStringBuffer(swTraceDeclNameLength - 1);
std::memcpy(swTraceStringBuffer.data(),
@@ -346,6 +353,11 @@
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 @@
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 @@
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 985c49e..833b73d 100644
--- a/src/profiling/ProfilingUtils.hpp
+++ b/src/profiling/ProfilingUtils.hpp
@@ -214,7 +214,7 @@
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 0225d75..8043351 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_TEST(packetBuffer3.get());
}
+BOOST_AUTO_TEST_CASE(ReadSwTraceMessageExceptionTest0)
+{
+ IPacketBufferPtr packetBuffer = std::make_unique<PacketBuffer>(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<PacketBuffer>(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 da30cef..5e9f6bd 100644
--- a/src/profiling/test/SendTimelinePacketTests.cpp
+++ b/src/profiling/test/SendTimelinePacketTests.cpp
@@ -72,7 +72,9 @@
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_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_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_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_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 74aefea..5aac77c 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 @@
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 08d29d0..ceb955d 100644
--- a/src/timelineDecoder/tests/TimelineTests.cpp
+++ b/src/timelineDecoder/tests/TimelineTests.cpp
@@ -114,7 +114,9 @@
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 cefb723..e7eb084 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 @@
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)