Skip to content
Open
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion Fw/Dp/DpContainer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,8 @@ Fw::SerializeStatus DpContainer::deserializeHeader() {
if (status == Fw::FW_SERIALIZE_OK) {
const FwSizeType requestedSize = sizeof this->m_userData;
FwSizeType receivedSize = requestedSize;
status = deserializer.deserializeTo(this->m_userData, receivedSize, Fw::Serialization::OMIT_LENGTH);
status = deserializer.deserializeTo(this->m_userData, sizeof(this->m_userData), receivedSize,
Fw::Serialization::OMIT_LENGTH);
if (receivedSize != requestedSize) {
status = Fw::FW_DESERIALIZE_SIZE_MISMATCH;
}
Expand Down
3 changes: 2 additions & 1 deletion Fw/Dp/test/util/DpContainerHeader.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,8 @@ struct DpContainerHeader {
// Deserialize the user data
DpContainerHeader::moveDeserToOffset(file, line, deserializer, DpContainer::Header::USER_DATA_OFFSET);
FwSizeType size = sizeof this->m_userData;
status = deserializer.deserializeTo(this->m_userData, size, Fw::Serialization::OMIT_LENGTH);
status = deserializer.deserializeTo(this->m_userData, sizeof(this->m_userData), size,
Fw::Serialization::OMIT_LENGTH);
DP_CONTAINER_HEADER_ASSERT_EQ(status, FW_SERIALIZE_OK);
DP_CONTAINER_HEADER_ASSERT_EQ(size, sizeof this->m_userData);
// Deserialize the data product state
Expand Down
2 changes: 1 addition & 1 deletion Fw/FilePacket/PathName.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ SerializeStatus FilePacket::PathName ::fromSerialBuffer(SerialBuffer& serialBuff
{
const U8* addrLeft = serialBuffer.getBuffAddrLeft();
U8 bytes[MAX_LENGTH];
const SerializeStatus status = serialBuffer.popBytes(bytes, this->m_length);
const SerializeStatus status = serialBuffer.popBytes(bytes, MAX_LENGTH, this->m_length);

if (status != FW_SERIALIZE_OK) {
return status;
Expand Down
5 changes: 3 additions & 2 deletions Fw/Log/AmpcsEvrLogPacket.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ SerializeStatus AmpcsEvrLogPacket::deserializeFrom(SerialBufferBase& buffer) {
SerializeStatus stat;

len = AMPCS_EVR_TASK_NAME_LEN;
stat = buffer.deserializeTo(this->m_taskName, len, true);
stat = buffer.deserializeTo(this->m_taskName, AMPCS_EVR_TASK_NAME_LEN, len, Fw::Serialization::OMIT_LENGTH);
if (stat != FW_SERIALIZE_OK) {
return stat;
}
Expand All @@ -69,7 +69,8 @@ SerializeStatus AmpcsEvrLogPacket::deserializeFrom(SerialBufferBase& buffer) {
}

FwSizeType size = buffer.getDeserializeSizeLeft();
stat = buffer.deserializeTo(this->m_logBuffer.getBuffAddr(), size, true);
stat = buffer.deserializeTo(this->m_logBuffer.getBuffAddr(), this->m_logBuffer.getCapacity(), size,
Fw::Serialization::OMIT_LENGTH);
if (stat == FW_SERIALIZE_OK) {
// Shouldn't fail
stat = this->m_logBuffer.setBuffLen(size);
Expand Down
3 changes: 2 additions & 1 deletion Fw/Log/LogPacket.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,8 @@ SerializeStatus LogPacket::deserializeFrom(SerialBufferBase& buffer, Fw::Endiann

// remainder of buffer must be telemetry value
FwSizeType size = buffer.getDeserializeSizeLeft();
stat = buffer.deserializeTo(this->m_logBuffer.getBuffAddr(), size, Fw::Serialization::OMIT_LENGTH);
stat = buffer.deserializeTo(this->m_logBuffer.getBuffAddr(), this->m_logBuffer.getCapacity(), size,
Fw::Serialization::OMIT_LENGTH);
if (stat == FW_SERIALIZE_OK) {
// Shouldn't fail
stat = this->m_logBuffer.setBuffLen(size);
Expand Down
6 changes: 4 additions & 2 deletions Fw/Tlm/TlmPacket.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,8 @@
}

// telemetry buffer
stat = this->m_tlmBuffer.deserializeTo(buffer.getBuffAddr(), bufferSize, Fw::Serialization::OMIT_LENGTH);
stat = this->m_tlmBuffer.deserializeTo(buffer.getBuffAddr(), buffer.getCapacity(), bufferSize,

Check warning

Code scanning / CodeQL

Unchecked function argument Warning

This use of parameter buffer has not been checked.

Check warning

Code scanning / CodeQL

Unchecked function argument Warning

This use of parameter bufferSize has not been checked.
Comment thread
github-advanced-security[bot] marked this conversation as resolved.
Fixed
Comment thread
github-advanced-security[bot] marked this conversation as resolved.
Fixed
Fw::Serialization::OMIT_LENGTH);
if (stat != Fw::FW_SERIALIZE_OK) {
return stat;
}
Expand Down Expand Up @@ -142,7 +143,8 @@
}
// deserialize the channel value entry buffers
FwSizeType size = buffer.getDeserializeSizeLeft();
stat = buffer.deserializeTo(this->m_tlmBuffer.getBuffAddr(), size, Fw::Serialization::OMIT_LENGTH);
stat = buffer.deserializeTo(this->m_tlmBuffer.getBuffAddr(), this->m_tlmBuffer.getCapacity(), size,
Fw::Serialization::OMIT_LENGTH);
if (stat == FW_SERIALIZE_OK) {
// Shouldn't fail
stat = this->m_tlmBuffer.setBuffLen(size);
Expand Down
4 changes: 2 additions & 2 deletions Fw/Types/SerialBuffer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,8 @@
return this->serializeFrom(const_cast<U8*>(addr), n, Fw::Serialization::OMIT_LENGTH);
}

SerializeStatus SerialBuffer ::popBytes(U8* const addr, FwSizeType n) {
return this->deserializeTo(addr, n, Fw::Serialization::OMIT_LENGTH);
SerializeStatus SerialBuffer ::popBytes(U8* const addr, FwSizeType buffCapacity, FwSizeType n) {
return this->deserializeTo(addr, buffCapacity, n, Fw::Serialization::OMIT_LENGTH);

Check warning

Code scanning / CodeQL

Unchecked function argument Warning

This use of parameter addr has not been checked.

Check warning

Code scanning / CodeQL

Unchecked function argument Warning

This use of parameter buffCapacity has not been checked.

Check warning

Code scanning / CodeQL

Unchecked function argument Warning

This use of parameter n has not been checked.
Comment thread
github-advanced-security[bot] marked this conversation as resolved.
Fixed
Comment thread
github-advanced-security[bot] marked this conversation as resolved.
Fixed
Comment thread
github-advanced-security[bot] marked this conversation as resolved.
Fixed
}

} // namespace Fw
5 changes: 3 additions & 2 deletions Fw/Types/SerialBuffer.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,9 @@ class SerialBuffer final : public LinearBufferBase {
);

//! Pop n bytes off the buffer
SerializeStatus popBytes(U8* const addr, //!< Address of bytes to pop
FwSizeType n //!< Number of bytes to pop
SerializeStatus popBytes(U8* const addr, //!< Address of bytes to pop
FwSizeType buffCapacity, //!< Capacity in bytes of addr
FwSizeType n //!< Number of bytes to pop
);

private:
Expand Down
37 changes: 19 additions & 18 deletions Fw/Types/Serializable.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -515,14 +515,19 @@
return FW_SERIALIZE_OK;
}

SerializeStatus LinearBufferBase::deserializeTo(U8* buff, Serializable::SizeType& length, Endianness endianMode) {
SerializeStatus LinearBufferBase::deserializeTo(U8* buff,
FwSizeType buffCapacity,
Serializable::SizeType& length,
Endianness endianMode) {
FwSizeType length_in_out = static_cast<FwSizeType>(length);
SerializeStatus status = this->deserializeTo(buff, length_in_out, Serialization::INCLUDE_LENGTH, endianMode);
SerializeStatus status =
this->deserializeTo(buff, buffCapacity, length_in_out, Serialization::INCLUDE_LENGTH, endianMode);

Check warning

Code scanning / CodeQL

Unchecked function argument Warning

This use of parameter buff has not been checked.

Check warning

Code scanning / CodeQL

Unchecked function argument Warning

This use of parameter buffCapacity has not been checked.

Check warning

Code scanning / CodeQL

Unchecked function argument Warning

This use of parameter endianMode has not been checked.
Comment thread
github-advanced-security[bot] marked this conversation as resolved.
Fixed
Comment thread
github-advanced-security[bot] marked this conversation as resolved.
Fixed
Comment thread
github-advanced-security[bot] marked this conversation as resolved.
Fixed
length = static_cast<Serializable::SizeType>(length_in_out);
return status;
}

SerializeStatus LinearBufferBase::deserializeTo(U8* buff,
FwSizeType buffCapacity,
Serializable::SizeType& length,
Serialization::t lengthMode,
Endianness endianMode) {
Expand All @@ -537,8 +542,8 @@
return stat;
}

// make sure it fits
if ((storedLength > this->getDeserializeSizeLeft()) or (storedLength > length)) {
// make sure it fits in both the source and the destination
if ((storedLength > this->getDeserializeSizeLeft()) or (storedLength > buffCapacity)) {
return FW_DESERIALIZE_SIZE_MISMATCH;
}

Expand All @@ -550,11 +555,16 @@
length = static_cast<FwSizeType>(storedLength);

} else {
// make sure enough is left
// make sure enough is left in the source
if (length > this->getDeserializeSizeLeft()) {
return FW_DESERIALIZE_SIZE_MISMATCH;
}

// make sure the destination buffer is large enough
if (length > buffCapacity) {
return FW_DESERIALIZE_SIZE_MISMATCH;
}

if (length > 0) {
FW_ASSERT(buff);
}
Expand Down Expand Up @@ -930,19 +940,10 @@
return this->deserializeTo(val);
}

// Deprecated method for backward compatibility
SerializeStatus LinearBufferBase::deserialize(U8* buff, FwSizeType& length, bool noLength) {
const Serialization::t mode = noLength ? Serialization::OMIT_LENGTH : Serialization::INCLUDE_LENGTH;
return this->deserializeTo(buff, length, mode);
}

SerializeStatus LinearBufferBase::deserialize(U8* buff, FwSizeType& length) {
return this->deserializeTo(buff, length, Serialization::INCLUDE_LENGTH);
}

SerializeStatus LinearBufferBase::deserialize(U8* buff, FwSizeType& length, Serialization::t mode) {
return this->deserializeTo(buff, length, mode);
}
// NOTE: The deprecated deserialize(U8* buff, ...) overloads have been removed.
// They could not be safely forwarded to deserializeTo() after the addition of
// the buffCapacity parameter. Callers must migrate to:
// deserializeTo(buff, buffCapacity, length, lengthMode)

SerializeStatus LinearBufferBase::deserialize(Serializable& val) {
return this->deserializeTo(val);
Expand Down
28 changes: 21 additions & 7 deletions Fw/Types/Serializable.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -456,10 +456,14 @@ class SerialBufferBase {
//! deserialization can be controlled via the endianMode parameter.
//!
//! \param buff Pointer to the buffer where deserialized data will be stored
//! \param buffCapacity Size in bytes of the destination buffer pointed to by buff
//! \param length Reference to store the actual number of bytes deserialized
//! \param endianMode Endianness mode for deserialization (default is Endianness::BIG)
//! \return SerializeStatus indicating the result of the operation
virtual SerializeStatus deserializeTo(U8* buff, FwSizeType& length, Endianness endianMode = Endianness::BIG) = 0;
virtual SerializeStatus deserializeTo(U8* buff,
FwSizeType buffCapacity,
FwSizeType& length,
Endianness endianMode = Endianness::BIG) = 0;

//! \brief Deserialize a byte buffer with optional length prefix
//!
Expand All @@ -470,11 +474,13 @@ class SerialBufferBase {
//! endianMode parameter.
//!
//! \param buff Pointer to the buffer where deserialized data will be stored
//! \param buffCapacity Size in bytes of the destination buffer pointed to by buff
//! \param length Reference to store the actual number of bytes deserialized
//! \param lengthMode Specifies whether length was included in serialization (INCLUDE_LENGTH or OMIT_LENGTH)
//! \param endianMode Endianness mode for deserialization (default is Endianness::BIG)
//! \return SerializeStatus indicating the result of the operation
virtual SerializeStatus deserializeTo(U8* buff,
FwSizeType buffCapacity,
FwSizeType& length,
Serialization::t lengthMode,
Endianness endianMode = Endianness::BIG) = 0;
Expand Down Expand Up @@ -1031,10 +1037,14 @@ class LinearBufferBase : public SerialBufferBase {
//! deserialization can be controlled via the endianMode parameter.
//!
//! \param buff Pointer to the buffer where deserialized data will be stored
//! \param buffCapacity Size in bytes of the destination buffer pointed to by buff
//! \param length Reference to store the actual number of bytes deserialized
//! \param endianMode Endianness mode for deserialization (default is Endianness::BIG)
//! \return SerializeStatus indicating the result of the operation
SerializeStatus deserializeTo(U8* buff, FwSizeType& length, Endianness endianMode = Endianness::BIG) override;
SerializeStatus deserializeTo(U8* buff,
FwSizeType buffCapacity,
FwSizeType& length,
Endianness endianMode = Endianness::BIG) override;

//! \brief Deserialize a byte buffer with optional length prefix
//!
Expand All @@ -1045,11 +1055,13 @@ class LinearBufferBase : public SerialBufferBase {
//! endianMode parameter.
//!
//! \param buff Pointer to the buffer where deserialized data will be stored
//! \param buffCapacity Size in bytes of the destination buffer pointed to by buff
//! \param length Reference to store the actual number of bytes deserialized
//! \param lengthMode Specifies whether length was included in serialization (INCLUDE_LENGTH or OMIT_LENGTH)
//! \param endianMode Endianness mode for deserialization (default is Endianness::BIG)
//! \return SerializeStatus indicating the result of the operation
SerializeStatus deserializeTo(U8* buff,
FwSizeType buffCapacity,
FwSizeType& length,
Serialization::t lengthMode,
Endianness endianMode = Endianness::BIG) override;
Expand Down Expand Up @@ -1322,12 +1334,14 @@ class LinearBufferBase : public SerialBufferBase {
DEPRECATED(SerializeStatus deserialize(F64& val), "Use deserializeTo(F64& val) instead");
DEPRECATED(SerializeStatus deserialize(bool& val), "Use deserializeTo(bool& val) instead");
DEPRECATED(SerializeStatus deserialize(void*& val), "Use deserializeTo(void*& val) instead");
DEPRECATED(SerializeStatus deserialize(U8* buff, FwSizeType& length, bool noLength),
"Use deserialize(U8* buff, FwSizeType& length, Serialization::t mode) instead");
DEPRECATED(
SerializeStatus deserialize(U8* buff, FwSizeType& length, bool noLength),
"Use deserializeTo(U8* buff, FwSizeType buffCapacity, FwSizeType& length, Serialization::t mode) instead");
DEPRECATED(SerializeStatus deserialize(U8* buff, FwSizeType& length),
"Use deserializeTo(U8* buff, FwSizeType& length) instead");
DEPRECATED(SerializeStatus deserialize(U8* buff, FwSizeType& length, Serialization::t mode),
"Use deserializeTo(U8* buff, FwSizeType& length, Serialization::t mode) instead");
"Use deserializeTo(U8* buff, FwSizeType buffCapacity, FwSizeType& length) instead");
DEPRECATED(
SerializeStatus deserialize(U8* buff, FwSizeType& length, Serialization::t mode),
"Use deserializeTo(U8* buff, FwSizeType buffCapacity, FwSizeType& length, Serialization::t mode) instead");
DEPRECATED(SerializeStatus deserialize(Serializable& val), "Use deserializeTo(Serializable& val) instead");

#ifdef BUILD_UT
Expand Down
3 changes: 2 additions & 1 deletion Fw/Types/StringBase.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,8 @@
// Deserialize length
// Fail if length exceeds max size (the initial value of actualSize)
// Otherwise deserialize length bytes and set actualSize to length
SerializeStatus stat = buffer.deserializeTo(reinterpret_cast<U8*>(raw), actualSize, Serialization::INCLUDE_LENGTH);
SerializeStatus stat = buffer.deserializeTo(reinterpret_cast<U8*>(raw), static_cast<FwSizeType>(maxSize),

Check warning

Code scanning / CodeQL

Unchecked function argument Warning

This use of parameter buffer has not been checked.
actualSize, Serialization::INCLUDE_LENGTH);
if (stat == FW_SERIALIZE_OK) {
// Deserialization succeeded: null-terminate string at actual size
FW_ASSERT(actualSize <= maxSize, static_cast<FwAssertArgType>(actualSize),
Expand Down
2 changes: 1 addition & 1 deletion Ref/RecvBuffApp/RecvBuffComponentImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ void RecvBuffImpl::Data_handler(FwIndexType portNum, Drv::DataBuffer& buff) {
// deserialize data
U8 testData[24] = {0};
FwSizeType size = sizeof(testData);
stat = buff.deserializeTo(testData, size);
stat = buff.deserializeTo(testData, sizeof(testData), size, Fw::Serialization::OMIT_LENGTH);
FW_ASSERT(stat == Fw::FW_SERIALIZE_OK, static_cast<FwAssertArgType>(stat));
// deserialize checksum
U32 csum = 0;
Expand Down
2 changes: 2 additions & 0 deletions Svc/Ccsds/SpacePacketFramer/SpacePacketFramer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,8 @@ void SpacePacketFramer ::dataIn_handler(FwIndexType portNum, Fw::Buffer& data, c
FW_ASSERT(status == Fw::FW_SERIALIZE_OK, status);
status = frameSerializer.serializeFrom(data.getData(), data.getSize(), Fw::Serialization::OMIT_LENGTH);
FW_ASSERT(status == Fw::FW_SERIALIZE_OK, status);
// Trim to actual frame size in case allocator returned a larger buffer
frameBuffer.setSize(static_cast<Fw::Buffer::SizeType>(frameSize));

this->dataOut_out(0, frameBuffer, context);
this->dataReturnOut_out(0, data, context); // return ownership of the original data buffer
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,11 @@ TEST(SpacePacketFramer, testNominalFraming) {
tester.testNominalFraming();
}

TEST(SpacePacketFramer, OversizedAllocatorBufferIsTrimmed) {
Svc::Ccsds::SpacePacketFramerTester tester;
tester.testOversizedAllocatorBufferIsTrimmed();
}

int main(int argc, char** argv) {
::testing::InitGoogleTest(&argc, argv);
return RUN_ALL_TESTS();
Expand Down
27 changes: 27 additions & 0 deletions Svc/Ccsds/SpacePacketFramer/test/ut/SpacePacketFramerTester.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,33 @@ void SpacePacketFramerTester::testNominalFraming() {
ASSERT_EQ(this->fromPortHistory_dataReturnOut->at(0).data.getSize(), sizeof(payload));
}

void SpacePacketFramerTester ::testOversizedAllocatorBufferIsTrimmed() {
U8 payload[16];
for (U32 i = 0; i < sizeof(payload); ++i) {
payload[i] = static_cast<U8>(STest::Random::lowerUpper(0, 0xFF));
}
Fw::Buffer data(payload, sizeof(payload));
ComCfg::FrameContext context;
context.set_apid(static_cast<ComCfg::Apid::T>(0x01));
this->m_nextSeqCount = 0;

// Signal the allocator handler to return a larger-than-needed buffer,
// simulating a BufferManager bin that is bigger than the exact packet size.
this->m_useOversizedAlloc = true;
this->invoke_to_dataIn(0, data, context);
this->m_useOversizedAlloc = false;

ASSERT_from_dataOut_SIZE(1);
ASSERT_from_dataReturnOut_SIZE(1);

const FwSizeType expectedFrameSize = sizeof(payload) + SpacePacketHeader::SERIALIZED_SIZE;

Fw::Buffer outBuffer = this->fromPortHistory_dataOut->at(0).data;
// If setSize() is missing from SpacePacketFramer, getSize() returns the
// oversized allocation (2 * expectedFrameSize) and this assertion fails.
ASSERT_EQ(outBuffer.getSize(), expectedFrameSize);
}

// ----------------------------------------------------------------------
// Output port handler overrides
// ----------------------------------------------------------------------
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ class SpacePacketFramerTester final : public SpacePacketFramerGTestBase {
void testComStatusPassthrough();
void testDataReturnPassthrough();
void testNominalFraming();
void testOversizedAllocatorBufferIsTrimmed();

private:
// ----------------------------------------------------------------------
Expand Down Expand Up @@ -80,6 +81,12 @@ class SpacePacketFramerTester final : public SpacePacketFramerGTestBase {
U8 m_internalDataBuffer[SpacePacketHeader::SERIALIZED_SIZE + MAX_TEST_PACKET_DATA_SIZE];

U16 m_nextSeqCount; // Sequence count to be returned by getApidSeqCount output port

// Backing store for the oversized allocation path
U8 m_oversizedDataBuffer[512];

// Flag read by from_bufferAllocate_handler to choose which path to take
bool m_useOversizedAlloc = false;
};

} // namespace Ccsds
Expand Down
3 changes: 2 additions & 1 deletion Svc/CmdSequencer/FPrimeSequence.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -299,7 +299,8 @@ Fw::SerializeStatus CmdSequencerComponentImpl::FPrimeSequence ::copyCommand(Fw::
FwSizeType size = recordSize;
Fw::SerializeStatus status = comBuffer.setBuffLen(recordSize);
FW_ASSERT(status == Fw::FW_SERIALIZE_OK, status);
status = buffer.deserializeTo(comBuffer.getBuffAddr(), size, Fw::Serialization::OMIT_LENGTH);
status =
buffer.deserializeTo(comBuffer.getBuffAddr(), comBuffer.getCapacity(), size, Fw::Serialization::OMIT_LENGTH);
return status;
}

Expand Down
3 changes: 2 additions & 1 deletion Svc/CmdSequencer/formats/AMPCSSequence.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -338,7 +338,8 @@ Fw::SerializeStatus AMPCSSequence ::translateCommand(Fw::ComBuffer& comBuffer, c
U8* const addr = comBuffer.getBuffAddr();
FW_ASSERT(addr != nullptr);
// true means "don't serialize the length"
status = buffer.deserializeTo(&addr[fixedBuffLen], size, Fw::Serialization::OMIT_LENGTH);
status = buffer.deserializeTo(&addr[fixedBuffLen], comBuffer.getCapacity() - fixedBuffLen, size,
Fw::Serialization::OMIT_LENGTH);
return status;
}

Expand Down
Loading
Loading