Skip to content

Commit 1bf11c5

Browse files
web-flowweb-flow
committed
Fix #11765 FN: minsize not checked for string literal, buffer access out of bounds not found
1 parent b2d8361 commit 1bf11c5

4 files changed

Lines changed: 55 additions & 24 deletions

File tree

‎lib/checkbufferoverrun.cpp‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -558,7 +558,7 @@ void CheckBufferOverrunImpl::pointerArithmeticError(const Token *tok, const Toke
558558

559559
//---------------------------------------------------------------------------
560560

561-
ValueFlow::Value CheckBufferOverrunImpl::getBufferSize(const Token *bufTok, const Settings& settings) const
561+
ValueFlow::Value CheckBufferOverrunImpl::getBufferSize(const Token *bufTok, const Settings& settings, bool checkStringLiteral) const
562562
{
563563
if (!bufTok->valueType())
564564
return ValueFlow::Value(-1);
@@ -604,6 +604,14 @@ ValueFlow::Value CheckBufferOverrunImpl::getBufferSize(const Token *bufTok, cons
604604
return bufSizeVal;
605605
}
606606
}
607+
} else if (checkStringLiteral && bufTok->tokType() == Token::Type::eString && bufTok->valueType()) {
608+
ValueFlow::Value strSizeVal;
609+
strSizeVal.valueType = ValueFlow::Value::ValueType::BUFFER_SIZE;
610+
const size_t charSize = bufTok->valueType()->getSizeOf(settings, ValueType::Accuracy::ExactOrZero, ValueType::SizeOf::Pointee);
611+
if (charSize > 0) {
612+
strSizeVal.intvalue = Token::getStrArraySize(bufTok) * charSize;
613+
return strSizeVal;
614+
}
607615
}
608616
}
609617

@@ -715,7 +723,8 @@ void CheckBufferOverrunImpl::bufferOverflow()
715723
if (argtok->valueType() && argtok->valueType()->pointer == 0)
716724
continue;
717725
// TODO: strcpy(buf+10, "hello");
718-
ValueFlow::Value bufferSize = getBufferSize(argtok, mSettings);
726+
const bool strz = mSettings.library.isargstrz(tok, argnr + 1);
727+
ValueFlow::Value bufferSize = getBufferSize(argtok, mSettings, !strz);
719728
if (bufferSize.intvalue <= 0)
720729
continue;
721730
// buffer size == 1 => do not warn for dynamic memory

‎lib/checkbufferoverrun.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,7 @@ class CPPCHECKLIB CheckBufferOverrunImpl : public CheckImpl
129129
void objectIndex();
130130
void objectIndexError(const Token *tok, const ValueFlow::Value *v, bool known);
131131

132-
ValueFlow::Value getBufferSize(const Token *bufTok, const Settings& settings) const;
132+
ValueFlow::Value getBufferSize(const Token *bufTok, const Settings& settings, bool checkStringLiteral = false) const;
133133

134134
// CTU
135135
static bool isCtuUnsafeBufferUsage(const Settings &settings, const Token *argtok, CTU::FileInfo::Value *offset, int type);

‎test/cfg/posix.c‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -719,6 +719,7 @@ size_t bufferAccessOutOfBounds_strnlen(const char *s, size_t maxlen)
719719
size_t len = strnlen(buf,2);
720720
// cppcheck-suppress bufferAccessOutOfBounds
721721
len+=strnlen(buf,3);
722+
len+=strnlen("ab",3);
722723
return len;
723724
}
724725

@@ -1606,4 +1607,4 @@ void pthread_define_test()
16061607
(void)rwlock;
16071608
PTHREAD_SCOPE_PROCESS;
16081609
PTHREAD_SCOPE_SYSTEM;
1609-
}
1610+
}

‎test/testbufferoverrun.cpp‎

Lines changed: 41 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -229,6 +229,7 @@ class TestBufferOverrun : public TestFixture {
229229
TEST_CASE(buffer_overrun_36);
230230
TEST_CASE(buffer_overrun_37);
231231
TEST_CASE(buffer_overrun_38);
232+
TEST_CASE(buffer_overrun_39);
232233
TEST_CASE(buffer_overrun_errorpath);
233234
TEST_CASE(buffer_overrun_bailoutIfSwitch); // ticket #2378 : bailoutIfSwitch
234235
TEST_CASE(buffer_overrun_function_array_argument);
@@ -3484,23 +3485,6 @@ class TestBufferOverrun : public TestFixture {
34843485
" (void)strxfrm(dest,src,3);\n" // <<
34853486
"}\n");
34863487
ASSERT_EQUALS("[test.cpp:6:19]: (error) Buffer is accessed out of bounds: dest [bufferAccessOutOfBounds]\n", errout_str());
3487-
// source size is too small
3488-
check("void f(void) {\n"
3489-
" const char src[2] = \"ab\";\n"
3490-
" char dest[3] = \"abc\";\n"
3491-
" (void)strxfrm(dest,src,1);\n"
3492-
" (void)strxfrm(dest,src,2);\n"
3493-
" (void)strxfrm(dest,src,3);\n" // <<
3494-
"}\n");
3495-
ASSERT_EQUALS("[test.cpp:6:24]: (error) Buffer is accessed out of bounds: src [bufferAccessOutOfBounds]\n", errout_str());
3496-
// source size is too small
3497-
check("void f(void) {\n"
3498-
" const char src[1] = \"a\";\n"
3499-
" char dest[3] = \"abc\";\n"
3500-
" (void)strxfrm(dest,src,1);\n"
3501-
" (void)strxfrm(dest,src,2);\n" // <<
3502-
"}\n");
3503-
ASSERT_EQUALS("[test.cpp:5:24]: (error) Buffer is accessed out of bounds: src [bufferAccessOutOfBounds]\n", errout_str());
35043488
}
35053489

35063490
void buffer_overrun_33() { // #2019
@@ -3638,6 +3622,44 @@ class TestBufferOverrun : public TestFixture {
36383622
ASSERT_EQUALS("", errout_str());
36393623
}
36403624

3625+
void buffer_overrun_39() { // #11765
3626+
check("void f() {\n"
3627+
" char buf[10];\n"
3628+
" memcpy(buf, \"abc\", 4);\n"
3629+
"}\n");
3630+
ASSERT_EQUALS("", errout_str());
3631+
3632+
check("void f() {\n"
3633+
" char buf[128];\n"
3634+
" memcpy(buf, \"abc\", 5);\n"
3635+
"}\n");
3636+
ASSERT_EQUALS("[test.cpp:3:17]: (warning) Buffer is accessed out of bounds: \"abc\" [bufferAccessOutOfBounds]\n",
3637+
errout_str());
3638+
3639+
check("void f() {\n"
3640+
" char buf[128];\n"
3641+
" memcpy(buf, L\"abc\", 4 * sizeof(wchar_t));\n"
3642+
"}\n");
3643+
ASSERT_EQUALS("", errout_str());
3644+
3645+
check("void f() {\n"
3646+
" char buf[128];\n"
3647+
" memcpy(buf, L\"abc\", 4 * sizeof(wchar_t) + 1);\n"
3648+
"}\n");
3649+
ASSERT_EQUALS("[test.cpp:3:17]: (warning) Buffer is accessed out of bounds: L\"abc\" [bufferAccessOutOfBounds]\n",
3650+
errout_str());
3651+
3652+
check("int f(char* s) {\n"
3653+
" return memcmp(s, \"\\0\\0\\0\", 3);\n"
3654+
"}\n");
3655+
ASSERT_EQUALS("", errout_str());
3656+
3657+
check("int f(const char16_t* buf) {\n"
3658+
" return memcmp(buf, u\"abc\", 4 * sizeof(char16_t));\n"
3659+
"}\n");
3660+
ASSERT_EQUALS("", errout_str());
3661+
}
3662+
36413663
void buffer_overrun_errorpath() {
36423664
setMultiline();
36433665
Settings s = settings0;
@@ -4618,9 +4640,8 @@ class TestBufferOverrun : public TestFixture {
46184640
check("void f() {\n"
46194641
" mymemset(\"abc\", 0, 20);\n"
46204642
"}\n", settings);
4621-
TODO_ASSERT_EQUALS("[test.cpp:2]: (error) Buffer is accessed out of bounds.\n",
4622-
"",
4623-
errout_str());
4643+
ASSERT_EQUALS("[test.cpp:2:12]: (warning) Buffer is accessed out of bounds: \"abc\" [bufferAccessOutOfBounds]\n",
4644+
errout_str());
46244645

46254646
check("void f() {\n"
46264647
" mymemset(temp, \"abc\", 4);\n"

0 commit comments

Comments
 (0)