Fix wrong offset calculation in SOME/IP-TP (#1898)

* Fix the bug in `SomeIpTpLayer::getOffset()` and `SomeIpTpLayer::setOffset()`

* Fix formatting

Author: seladb

Packet++/header/SomeIpLayer.h

@@ -344,6 +344,7 @@ namespace pcpp
 		/// Set the Offset. Already has to be in multiples of 16 bytes.
 		/// If 32 bytes have already been transmitted, the offset has to be set to 2.
 		/// @param[in] offset Offset to set. Already has to be in multiples of 16 bytes.
+		/// @throw std::invalid_argument if offset isn't a multiple of 16
 		void setOffset(uint32_t offset);
 
 		/// Get the More Segments Flag

Packet++/src/SomeIpLayer.cpp

@@ -306,12 +306,18 @@ namespace pcpp
 
 	uint32_t SomeIpTpLayer::getOffset() const
 	{
-		return (be32toh(getSomeIpTpHeader()->offsetAndFlag) & SOMEIP_TP_OFFSET_MASK) >> 4;
+		return (be32toh(getSomeIpTpHeader()->offsetAndFlag) & SOMEIP_TP_OFFSET_MASK);
 	}
 
 	void SomeIpTpLayer::setOffset(uint32_t offset)
 	{
-		uint32_t val = (offset << 4) | (be32toh(getSomeIpTpHeader()->offsetAndFlag) & ~SOMEIP_TP_OFFSET_MASK);
+		if ((offset & SOMEIP_TP_OFFSET_MASK) != offset)
+		{
+			throw std::invalid_argument("Invalid offset - should be a multiple of 16");
+		}
+
+		uint32_t val =
+		    (offset & SOMEIP_TP_OFFSET_MASK) | (be32toh(getSomeIpTpHeader()->offsetAndFlag) & ~SOMEIP_TP_OFFSET_MASK);
 		getSomeIpTpHeader()->offsetAndFlag = htobe32(val);
 	}
 

Tests/Packet++Test/Tests/SomeIpTests.cpp

@@ -233,7 +233,7 @@ PTF_TEST_CASE(SomeIpTpParsingTest)
 	PTF_ASSERT_EQUAL(someIpTpLayer2->getMessageType(), pcpp::SomeIpLayer::MsgType::TP_REQUEST_NO_RETURN, enumclass);
 	PTF_ASSERT_EQUAL(someIpTpLayer2->getMessageTypeAsInt(), (uint8_t)pcpp::SomeIpLayer::MsgType::TP_REQUEST_NO_RETURN);
 	PTF_ASSERT_EQUAL(someIpTpLayer2->getReturnCode(), 0);
-	PTF_ASSERT_EQUAL(someIpTpLayer2->getOffset() * 16, 91872);
+	PTF_ASSERT_EQUAL(someIpTpLayer2->getOffset(), 91872);
 	PTF_ASSERT_FALSE(someIpTpLayer2->getMoreSegmentsFlag());
 	PTF_ASSERT_EQUAL(someIpTpLayer2->getPduPayloadSize(), 225);
 	PTF_ASSERT_EQUAL(someIpTpLayer2->getPduPayload()[0], 0xab);
@@ -284,8 +284,8 @@ PTF_TEST_CASE(SomeIpTpCreationTest)
 	pcpp::EthLayer ethLayer2(ethLayer1);
 	pcpp::IPv4Layer ipLayer2(ipLayer1);
 	pcpp::UdpLayer udpLayer2(udpLayer1);
-	pcpp::SomeIpTpLayer someIpTpLayer2(0xd05f, 0x8001, 0, 0, 1, pcpp::SomeIpLayer::MsgType::REQUEST_NO_RETURN, 0,
-	                                   91872 / 16, false, data2, dataLen2);
+	pcpp::SomeIpTpLayer someIpTpLayer2(0xd05f, 0x8001, 0, 0, 1, pcpp::SomeIpLayer::MsgType::REQUEST_NO_RETURN, 0, 91872,
+	                                   false, data2, dataLen2);
 
 	pcpp::Packet someIpTpPacket2(500);
 	PTF_ASSERT_TRUE(someIpTpPacket2.addLayer(&ethLayer2));
@@ -306,15 +306,18 @@ PTF_TEST_CASE(SomeIpTpEditTest)
 	const size_t dataLen2 = 225;
 	uint8_t data2[dataLen2] = { 0 };
 
-	pcpp::SomeIpTpLayer someIpTpLayer(0x6059, 0x410c, 0x3, 0xa, 0x5, pcpp::SomeIpLayer::MsgType::REQUEST, 0, 91872 / 16,
+	pcpp::SomeIpTpLayer someIpTpLayer(0x6059, 0x410c, 0x3, 0xa, 0x5, pcpp::SomeIpLayer::MsgType::REQUEST, 0, 91872,
 	                                  true, data2, dataLen2);
-	someIpTpLayer.setOffset(123);
+	someIpTpLayer.setOffset(1968);
 
-	PTF_ASSERT_EQUAL(someIpTpLayer.getOffset(), 123);
+	PTF_ASSERT_EQUAL(someIpTpLayer.getOffset(), 1968);
 	PTF_ASSERT_TRUE(someIpTpLayer.getMoreSegmentsFlag());
 
 	someIpTpLayer.setMoreSegmentsFlag(false);
 
-	PTF_ASSERT_EQUAL(someIpTpLayer.getOffset(), 123);
+	PTF_ASSERT_EQUAL(someIpTpLayer.getOffset(), 1968);
 	PTF_ASSERT_FALSE(someIpTpLayer.getMoreSegmentsFlag());
+
+	PTF_ASSERT_RAISES(someIpTpLayer.setOffset(17), std::invalid_argument,
+	                  "Invalid offset - should be a multiple of 16");
 }