Skip to content

Commit 64a06e0

Browse files
authored
Fix: JsonLogFileReader may cause a crash due to buffer overflow during reading. (alibaba#2246)
* fix and add ut * polish * polish
1 parent 1b7df94 commit 64a06e0

2 files changed

Lines changed: 28 additions & 1 deletion

File tree

core/file_server/reader/JsonLogFileReader.cpp

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ int32_t JsonLogFileReader::RemoveLastIncompleteLog(char* buffer,
3535
break;
3636
}
3737
// else impossible to be a valid json line, advance and skip
38-
char* pos = strchr(buffer + beginIdx, '\n');
38+
char* pos = static_cast<char*>(memchr(buffer + beginIdx, '\n', size - beginIdx));
3939
if (pos == NULL) {
4040
break;
4141
}
@@ -45,6 +45,9 @@ int32_t JsonLogFileReader::RemoveLastIncompleteLog(char* buffer,
4545
beginIdx = endIdx + 1;
4646
buffer[endIdx] = '\0';
4747
} while (beginIdx < size);
48+
if (beginIdx > size) {
49+
beginIdx = size;
50+
}
4851
readBytes = beginIdx;
4952

5053
if (allowRollback) {

core/unittest/reader/JsonLogFileReaderUnittest.cpp

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -517,6 +517,30 @@ void RemoveLastIncompleteLogUnittest::TestRemoveLastIncompleteLogNotValidJson()
517517
APSARA_TEST_EQUAL_FATAL(std::string(testLog.data(), matchSize), expectMatch);
518518
APSARA_TEST_EQUAL_FATAL(0, rollbackLineFeedCount);
519519
}
520+
{ // test invalid \n at the end of buffer
521+
std::string firstLog = R"({
522+
"key": {
523+
"nested_key": "second value"
524+
}
525+
})";
526+
std::string notjson = "not a json at all.\nnot a json at all.";
527+
std::string testLog = firstLog + '\n' + notjson;
528+
int32_t logSize = testLog.size();
529+
testLog += "\nnot a json at all.";
530+
std::replace(notjson.begin(), notjson.end(), '\n', '\0');
531+
size_t pos = notjson.rfind("not a json at all.");
532+
if (pos != std::string::npos) {
533+
notjson = notjson.substr(0, pos);
534+
}
535+
std::string expectMatch = firstLog + '\0' + notjson;
536+
;
537+
int32_t rollbackLineFeedCount = 0;
538+
size_t matchSize = mLogFileReader->RemoveLastIncompleteLog(
539+
const_cast<char*>(testLog.data()), logSize, rollbackLineFeedCount);
540+
APSARA_TEST_EQUAL_FATAL(expectMatch.size(), matchSize);
541+
APSARA_TEST_EQUAL_FATAL(std::string(testLog.data(), matchSize), expectMatch);
542+
APSARA_TEST_EQUAL_FATAL(1, rollbackLineFeedCount);
543+
}
520544
}
521545

522546
void RemoveLastIncompleteLogUnittest::TestRemoveLastIncompleteLogNotValidJsonNoRollback() {

0 commit comments

Comments
 (0)