fsfw/fsfw
fsfw/fsfw
10
3
Fork 6
Code Issues 31 Pull Requests 4 Releases 10 Wiki Activity

out of bounds access in DLE encoder #492

Merged
mohr merged 8 commits from mueller/dle-possible-bugfix into development 2021-10-04 14:43:54 +02:00
Conversation 2 Commits 8 Files Changed 2 +29 -22
2 changed files with 29 additions and 22 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

46
src/fsfw/globalfunctions/DleEncoder.cpp
View File

@ -165,11 +165,9 @@ ReturnValue_t DleEncoder::decodeStreamEscaped(const uint8_t *sourceStream, size_
if (sourceStream[encodedIndex++] != STX_CHAR) { if (sourceStream[encodedIndex++] != STX_CHAR) {
return DECODING_ERROR; return DECODING_ERROR;
} }
while ((encodedIndex < sourceStreamLen) while ((encodedIndex < sourceStreamLen) and (decodedIndex < maxDestStreamlen)) {
and (decodedIndex < maxDestStreamlen) switch(sourceStream[encodedIndex]) {
and (sourceStream[encodedIndex] != ETX_CHAR) case(DLE_CHAR): {
and (sourceStream[encodedIndex] != STX_CHAR)) {
if (sourceStream[encodedIndex] == DLE_CHAR) {
if(encodedIndex + 1 >= sourceStreamLen) { if(encodedIndex + 1 >= sourceStreamLen) {
//reached the end of the sourceStream //reached the end of the sourceStream
*readLen = sourceStreamLen; *readLen = sourceStreamLen;
@ -197,29 +195,33 @@ ReturnValue_t DleEncoder::decodeStreamEscaped(const uint8_t *sourceStream, size_
} }
} }
++encodedIndex; ++encodedIndex;
break;
} }
else { case(STX_CHAR): {
*readLen = encodedIndex;
mohr marked this conversation as resolved Outdated
mohr commented 2021-10-01 13:11:05 +02:00
Outdated
Review
Copy Link

I think this should be *readLen = encodedIndex;, we should preserve the STX as it might start a new, valid frame.

I think this should be `*readLen = encodedIndex;`, we should preserve the `STX` as it might start a new, valid frame.
return DECODING_ERROR;
}
case(ETX_CHAR): {
*readLen = ++encodedIndex;
*decodedLen = decodedIndex;
return RETURN_OK;
}
default: {
destStream[decodedIndex] = sourceStream[encodedIndex]; destStream[decodedIndex] = sourceStream[encodedIndex];
break;
}
} }
++encodedIndex; ++encodedIndex;
++decodedIndex; ++decodedIndex;
} }
if (sourceStream[encodedIndex] != ETX_CHAR) {
if(decodedIndex == maxDestStreamlen) { if(decodedIndex == maxDestStreamlen) {
//so far we did not find anything wrong here, so let user try again //so far we did not find anything wrong here, so let user try again
*readLen = 0; *readLen = 0;
return STREAM_TOO_SHORT; return STREAM_TOO_SHORT;
} } else {
else { *readLen = encodedIndex;
*readLen = ++encodedIndex; return DECODING_ERROR;
return DECODING_ERROR;
}
}
else {
*readLen = ++encodedIndex;
*decodedLen = decodedIndex;
return RETURN_OK;
} }
} }

5
tests/src/fsfw_tests/unit/globalfunctions/testDleEncoder.cpp
View File

@ -218,5 +218,10 @@ TEST_CASE("DleEncoder" , "[DleEncoder]") {
REQUIRE(result == static_cast<int>(DleEncoder::DECODING_ERROR)); REQUIRE(result == static_cast<int>(DleEncoder::DECODING_ERROR));
dleEncoder.setEscapeMode(true); dleEncoder.setEscapeMode(true);
testArray1EncodedFaulty = TEST_ARRAY_1_ENCODED_ESCAPED;
testArray1EncodedFaulty[5] = 0;
result = dleEncoder.decode(testArray1EncodedFaulty.data(), testArray1EncodedFaulty.size(),
&readLen, buffer.data(), buffer.size(), &encodedLen);
REQUIRE(result == static_cast<int>(DleEncoder::DECODING_ERROR));
} }
} }