From e649d45e3e65226a380c51218799bb50129b852a Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Tue, 16 Aug 2022 18:37:51 +0200 Subject: [PATCH] better size checks --- .../devicedefinitions/PlocMPSoCDefinitions.h | 62 ++++++++++++++----- .../PlocSupervisorDefinitions.h | 10 ++- 2 files changed, 54 insertions(+), 18 deletions(-) diff --git a/linux/devices/devicedefinitions/PlocMPSoCDefinitions.h b/linux/devices/devicedefinitions/PlocMPSoCDefinitions.h index b3b456d8..f92cfd22 100644 --- a/linux/devices/devicedefinitions/PlocMPSoCDefinitions.h +++ b/linux/devices/devicedefinitions/PlocMPSoCDefinitions.h @@ -237,7 +237,7 @@ class TcMemRead : public TcBase { uint16_t memLen = 0; ReturnValue_t lengthCheck(size_t commandDataLen) { - if (commandDataLen != COMMAND_LENGTH) { + if (commandDataLen != COMMAND_LENGTH or checkPayloadLen() != HasReturnvaluesIF::RETURN_OK) { return INVALID_LENGTH; } return HasReturnvaluesIF::RETURN_OK; @@ -264,10 +264,14 @@ class TcMemWrite : public TcBase { if (result != HasReturnvaluesIF::RETURN_OK) { return result; } - std::memcpy(payloadStart, commandData, commandDataLen); uint16_t memLen = *(commandData + MEM_ADDRESS_SIZE) << 8 | *(commandData + MEM_ADDRESS_SIZE + 1); spParams.setPayloadLen(MIN_FIXED_PAYLOAD_LENGTH + memLen * 4); + result = checkPayloadLen(); + if(result != HasReturnvaluesIF::RETURN_OK) { + return result; + } + std::memcpy(payloadStart, commandData, commandDataLen); return result; } @@ -279,7 +283,6 @@ class TcMemWrite : public TcBase { static const size_t MIN_COMMAND_DATA_LENGTH = MIN_FIXED_PAYLOAD_LENGTH + 4; - ReturnValue_t lengthCheck(size_t commandDataLen) { if (commandDataLen < MIN_COMMAND_DATA_LENGTH) { sif::warning << "TcMemWrite: Length " << commandDataLen << " smaller than minimum " << @@ -310,10 +313,14 @@ class FlashFopen : public ploc::SpTcBase { ReturnValue_t createPacket(std::string filename, char accessMode_) { accessMode = accessMode_; size_t nameSize = filename.size(); + spParams.setPayloadLen(nameSize + sizeof(NULL_TERMINATOR) + sizeof(accessMode)); + ReturnValue_t result = checkPayloadLen(); + if(result != HasReturnvaluesIF::RETURN_OK) { + return result; + } std::memcpy(payloadStart, filename.c_str(), nameSize); *(spParams.buf + nameSize) = NULL_TERMINATOR; std::memcpy(payloadStart + nameSize + sizeof(NULL_TERMINATOR), &accessMode, sizeof(accessMode)); - spParams.setPayloadLen(nameSize + sizeof(NULL_TERMINATOR) + sizeof(accessMode)); updateSpFields(); return calcCrc(); } @@ -325,16 +332,20 @@ class FlashFopen : public ploc::SpTcBase { /** * @brief Class to help creation of flash fclose command. */ -class FlashFclose : public TcBase { +class FlashFclose : public ploc::SpTcBase { public: FlashFclose(ploc::SpTcParams params, uint16_t sequenceCount) - : TcBase(params, apid::TC_FLASHFCLOSE, sequenceCount) {} + : ploc::SpTcBase(params, apid::TC_FLASHFCLOSE, sequenceCount) {} ReturnValue_t createPacket(std::string filename) { size_t nameSize = filename.size(); + spParams.setPayloadLen(nameSize + sizeof(NULL_TERMINATOR)); + ReturnValue_t result = checkPayloadLen(); + if(result != HasReturnvaluesIF::RETURN_OK) { + return result; + } std::memcpy(payloadStart, filename.c_str(), nameSize); *(payloadStart + nameSize) = NULL_TERMINATOR; - spParams.setPayloadLen(nameSize + sizeof(NULL_TERMINATOR)); return calcCrc(); } }; @@ -354,6 +365,11 @@ class TcFlashWrite : public ploc::SpTcBase { sif::debug << "FlashWrite::createPacket: Command data too big" << std::endl; return HasReturnvaluesIF::RETURN_FAILED; } + spParams.setPayloadLen(static_cast(writeLen) + 4); + result = checkPayloadLen(); + if(result != HasReturnvaluesIF::RETURN_OK) { + return result; + } size_t serializedSize = 0; result = SerializeAdapter::serialize(&writeLen, payloadStart, &serializedSize, sizeof(writeLen), SerializeIF::Endianness::BIG); @@ -361,7 +377,6 @@ class TcFlashWrite : public ploc::SpTcBase { return result; } std::memcpy(payloadStart + sizeof(writeLen), writeData, writeLen); - spParams.setPayloadLen(static_cast(writeLen) + 4); updateSpFields(); auto res = checkSizeAndSerializeHeader(); if (res != result::OK) { @@ -384,11 +399,16 @@ class TcFlashDelete : public ploc::SpTcBase { ReturnValue_t buildPacket(std::string filename) { size_t nameSize = filename.size(); + spParams.setPayloadLen(nameSize + sizeof(NULL_TERMINATOR)); + auto res = checkPayloadLen(); + if(res != HasReturnvaluesIF::RETURN_OK) { + return res; + } std::memcpy(payloadStart, filename.c_str(), nameSize); *(payloadStart + nameSize) = NULL_TERMINATOR; - spParams.setPayloadLen(nameSize + sizeof(NULL_TERMINATOR)); + updateSpFields(); - auto res = checkSizeAndSerializeHeader(); + res = checkSizeAndSerializeHeader(); if (res != result::OK) { return res; } @@ -419,6 +439,7 @@ class TcReplayStart : public TcBase { protected: ReturnValue_t initPacket(const uint8_t* commandData, size_t commandDataLen) override { ReturnValue_t result = HasReturnvaluesIF::RETURN_OK; + spParams.setPayloadLen(commandDataLen); result = lengthCheck(commandDataLen); if (result != HasReturnvaluesIF::RETURN_OK) { return result; @@ -428,7 +449,6 @@ class TcReplayStart : public TcBase { return result; } std::memcpy(payloadStart, commandData, commandDataLen); - spParams.setPayloadLen(commandDataLen); return result; } @@ -438,7 +458,7 @@ class TcReplayStart : public TcBase { static const uint8_t ONCE = 1; ReturnValue_t lengthCheck(size_t commandDataLen) { - if (commandDataLen != COMMAND_DATA_LENGTH) { + if (commandDataLen != COMMAND_DATA_LENGTH or checkPayloadLen() != HasReturnvaluesIF::RETURN_OK) { sif::warning << "TcReplayStart: Command has invalid length " << commandDataLen << std::endl; return INVALID_LENGTH; } @@ -480,9 +500,13 @@ class TcDownlinkPwrOn : public TcBase { if (result != HasReturnvaluesIF::RETURN_OK) { return result; } + spParams.setPayloadLen(commandDataLen + sizeof(MAX_AMPLITUDE)); + result = checkPayloadLen(); + if (result != HasReturnvaluesIF::RETURN_OK) { + return result; + } std::memcpy(payloadStart, commandData, commandDataLen); std::memcpy(payloadStart + commandDataLen, &MAX_AMPLITUDE, sizeof(MAX_AMPLITUDE)); - spParams.setPayloadLen(commandDataLen + sizeof(MAX_AMPLITUDE)); return result; } @@ -547,13 +571,13 @@ class TcReplayWriteSeq : public TcBase { protected: ReturnValue_t initPacket(const uint8_t* commandData, size_t commandDataLen) override { ReturnValue_t result = HasReturnvaluesIF::RETURN_OK; + spParams.setPayloadLen(commandDataLen + sizeof(NULL_TERMINATOR)); result = lengthCheck(commandDataLen); if (result != HasReturnvaluesIF::RETURN_OK) { return result; } std::memcpy(payloadStart, commandData, commandDataLen); *(payloadStart + commandDataLen) = NULL_TERMINATOR; - spParams.setPayloadLen(commandDataLen + sizeof(NULL_TERMINATOR)); return result; } @@ -561,7 +585,8 @@ class TcReplayWriteSeq : public TcBase { static const size_t USE_DECODING_LENGTH = 1; ReturnValue_t lengthCheck(size_t commandDataLen) { - if (commandDataLen > USE_DECODING_LENGTH + MAX_FILENAME_SIZE) { + if (commandDataLen > USE_DECODING_LENGTH + MAX_FILENAME_SIZE or + checkPayloadLen() != HasReturnvaluesIF::RETURN_OK) { sif::warning << "TcReplayWriteSeq: Command has invalid length " << commandDataLen << std::endl; return INVALID_LENGTH; @@ -632,12 +657,17 @@ class TcCamcmdSend : public TcBase { return INVALID_LENGTH; } uint16_t dataLen = static_cast(commandDataLen + sizeof(CARRIAGE_RETURN)); + spParams.setPayloadLen(sizeof(dataLen) + commandDataLen + sizeof(CARRIAGE_RETURN)); + auto res = checkPayloadLen(); + if(res != HasReturnvaluesIF::RETURN_OK) { + return res; + } size_t size = sizeof(dataLen); SerializeAdapter::serialize(&dataLen, payloadStart, &size, sizeof(dataLen), SerializeIF::Endianness::BIG); std::memcpy(payloadStart + sizeof(dataLen), commandData, commandDataLen); *(payloadStart + sizeof(dataLen) + commandDataLen) = CARRIAGE_RETURN; - spParams.setPayloadLen(sizeof(dataLen) + commandDataLen + sizeof(CARRIAGE_RETURN)); + return HasReturnvaluesIF::RETURN_OK; } diff --git a/linux/devices/devicedefinitions/PlocSupervisorDefinitions.h b/linux/devices/devicedefinitions/PlocSupervisorDefinitions.h index 37275b10..8ed6778a 100644 --- a/linux/devices/devicedefinitions/PlocSupervisorDefinitions.h +++ b/linux/devices/devicedefinitions/PlocSupervisorDefinitions.h @@ -1101,7 +1101,7 @@ class WriteMemory : public ploc::SpTcBase { static const uint16_t META_DATA_LENGTH = 8; uint8_t n = 1; - void initPacket(uint8_t memoryId, uint32_t startAddr, uint16_t updateDataLen, + ReturnValue_t initPacket(uint8_t memoryId, uint32_t startAddr, uint16_t updateDataLen, uint8_t* updateData) { size_t serializedSize = 0; uint8_t* data = payloadStart; @@ -1113,7 +1113,6 @@ class WriteMemory : public ploc::SpTcBase { SerializeIF::Endianness::BIG); SerializeAdapter::serialize(&updateDataLen, &data, &serializedSize, sizeof(updateDataLen), SerializeIF::Endianness::BIG); - std::memcpy(data, updateData, updateDataLen); if (updateDataLen % 2 != 0) { spParams.setPayloadLen(META_DATA_LENGTH + updateDataLen + 1); // The data field must be two bytes aligned. Thus, in case the number of bytes to write is odd @@ -1122,6 +1121,13 @@ class WriteMemory : public ploc::SpTcBase { } else { spParams.setPayloadLen(META_DATA_LENGTH + updateDataLen); } + // To avoid crashes in this unexpected case + ReturnValue_t result = checkPayloadLen(); + if(result != HasReturnvaluesIF::RETURN_OK) { + return result; + } + std::memcpy(data, updateData, updateDataLen); + return HasReturnvaluesIF::RETURN_OK; } };