walkero
/
cppcheck
1
0
Fork 0
Code Issues Pull Requests Projects Releases 1 Wiki Activity

Fix #11765 FN: minsize not checked for string literal, buffer access out of bounds not found (#5154)

This commit is contained in:
chrchr-github 2023-07-08 14:46:32 +02:00 committed by GitHub
parent 49b79b7674
commit 9ad18f51af
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 41 additions and 34 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
cfg/std.cfg
View File

@ -5011,7 +5011,6 @@ The obsolete function 'gets' is called. With 'gets' you'll get a buffer overrun
<arg nr="3" direction="in"> <arg nr="3" direction="in">
<not-null/> <not-null/>
<not-uninit/> <not-uninit/>
<minsize type="argvalue" arg="4"/>
<strz/> <strz/>
</arg> </arg>
<arg nr="4" direction="in"> <arg nr="4" direction="in">
@ -5097,7 +5096,6 @@ The obsolete function 'gets' is called. With 'gets' you'll get a buffer overrun
<arg nr="3" direction="in"> <arg nr="3" direction="in">
<not-null/> <not-null/>
<not-uninit/> <not-uninit/>
<minsize type="argvalue" arg="4"/>
<strz/> <strz/>
</arg> </arg>
<arg nr="4" direction="in"> <arg nr="4" direction="in">
@ -5213,7 +5211,6 @@ The obsolete function 'gets' is called. With 'gets' you'll get a buffer overrun
<not-null/> <not-null/>
<not-uninit/> <not-uninit/>
<strz/> <strz/>
<minsize type="argvalue" arg="3"/>
</arg> </arg>
<arg nr="3" direction="in"> <arg nr="3" direction="in">
<not-uninit/> <not-uninit/>

19
lib/checkbufferoverrun.cpp
View File

@ -554,20 +554,25 @@ ValueFlow::Value CheckBufferOverrun::getBufferSize(const Token *bufTok) const
return *value; return *value;
} }
if (!var) MathLib::bigint dim = -1;
return ValueFlow::Value(-1); if (var) {
dim = std::accumulate(var->dimensions().cbegin(), var->dimensions().cend(), 1LL, [](MathLib::bigint i1, const Dimension &dim) {
const MathLib::bigint dim = std::accumulate(var->dimensions().cbegin(), var->dimensions().cend(), 1LL, [](MathLib::bigint i1, const Dimension &dim) {
return i1 * dim.num; return i1 * dim.num;
}); });
}
else if (bufTok->tokType() == Token::Type::eString) {
dim = Token::getStrLength(bufTok) + 1;
}
else
return ValueFlow::Value(-1);
ValueFlow::Value v; ValueFlow::Value v;
v.setKnown(); v.setKnown();
v.valueType = ValueFlow::Value::ValueType::BUFFER_SIZE; v.valueType = ValueFlow::Value::ValueType::BUFFER_SIZE;
if (var->isPointerArray()) if (var && var->isPointerArray())
v.intvalue = dim * mSettings->platform.sizeof_pointer; v.intvalue = dim * mSettings->platform.sizeof_pointer;
else if (var->isPointer()) else if (var && var->isPointer())
return ValueFlow::Value(-1); return ValueFlow::Value(-1);
else { else {
const MathLib::bigint typeSize = bufTok->valueType()->typeSize(mSettings->platform); const MathLib::bigint typeSize = bufTok->valueType()->typeSize(mSettings->platform);
@ -646,7 +651,7 @@ void CheckBufferOverrun::bufferOverflow()
argtok = argtok->astOperand2() ? argtok->astOperand2() : argtok->astOperand1(); argtok = argtok->astOperand2() ? argtok->astOperand2() : argtok->astOperand1();
while (Token::Match(argtok, ".|::")) while (Token::Match(argtok, ".|::"))
argtok = argtok->astOperand2(); argtok = argtok->astOperand2();
if (!argtok || !argtok->variable()) if (!argtok || (!argtok->variable() && argtok->tokType() != Token::Type::eString))
continue; continue;
if (argtok->valueType() && argtok->valueType()->pointer == 0) if (argtok->valueType() && argtok->valueType()->pointer == 0)
continue; continue;

3
test/cfg/std.c
View File

@ -219,14 +219,12 @@ void bufferAccessOutOfBounds(void)
strncpy_s(a,5,"abcd",5); strncpy_s(a,5,"abcd",5);
// string will be truncated, error is returned, but no buffer overflow // string will be truncated, error is returned, but no buffer overflow
strncpy_s(a,5,"abcde",6); strncpy_s(a,5,"abcde",6);
// TODO cppcheck-suppress bufferAccessOutOfBounds
strncpy_s(a,5,"a",6); strncpy_s(a,5,"a",6);
strncpy_s(a,5,"abcdefgh",4); strncpy_s(a,5,"abcdefgh",4);
// valid call // valid call
strncat_s(a,5,"1",2); strncat_s(a,5,"1",2);
// cppcheck-suppress bufferAccessOutOfBounds // cppcheck-suppress bufferAccessOutOfBounds
strncat_s(a,10,"1",2); strncat_s(a,10,"1",2);
// TODO cppcheck-suppress bufferAccessOutOfBounds
strncat_s(a,5,"1",5); strncat_s(a,5,"1",5);
fread(a,1,5,stdin); fread(a,1,5,stdin);
// cppcheck-suppress bufferAccessOutOfBounds // cppcheck-suppress bufferAccessOutOfBounds
@ -518,7 +516,6 @@ void nullpointer(int value)
wcstok(NULL,L"xyz",&pWcsUninit); wcstok(NULL,L"xyz",&pWcsUninit);
strxfrm(0,"foo",0); strxfrm(0,"foo",0);
// TODO: error message (#6306 and http://trac.cppcheck.net/changeset/d11eb4931aea51cf2cb74faccdcd2a3289b818d6/)
strxfrm(0,"foo",42); strxfrm(0,"foo",42);
wcsxfrm(0,L"foo",0); wcsxfrm(0,L"foo",0);
// TODO: error message when arg1==NULL and arg3!=0 #6306: https://trac.cppcheck.net/ticket/6306#comment:2 // TODO: error message when arg1==NULL and arg3!=0 #6306: https://trac.cppcheck.net/ticket/6306#comment:2

48
test/testbufferoverrun.cpp
View File

@ -238,6 +238,7 @@ private:
TEST_CASE(buffer_overrun_34); //#11035 TEST_CASE(buffer_overrun_34); //#11035
TEST_CASE(buffer_overrun_35); //#2304 TEST_CASE(buffer_overrun_35); //#2304
TEST_CASE(buffer_overrun_36); TEST_CASE(buffer_overrun_36);
TEST_CASE(buffer_overrun_37);
TEST_CASE(buffer_overrun_errorpath); TEST_CASE(buffer_overrun_errorpath);
TEST_CASE(buffer_overrun_bailoutIfSwitch); // ticket #2378 : bailoutIfSwitch TEST_CASE(buffer_overrun_bailoutIfSwitch); // ticket #2378 : bailoutIfSwitch
TEST_CASE(buffer_overrun_function_array_argument); TEST_CASE(buffer_overrun_function_array_argument);
@ -3324,23 +3325,6 @@ private:
" (void)strxfrm(dest,src,3);\n" // << " (void)strxfrm(dest,src,3);\n" // <<
"}"); "}");
ASSERT_EQUALS("[test.cpp:6]: (error) Buffer is accessed out of bounds: dest\n", errout.str()); ASSERT_EQUALS("[test.cpp:6]: (error) Buffer is accessed out of bounds: dest\n", errout.str());
// source size is too small
check("void f(void) {\n"
" const char src[2] = \"ab\";\n"
" char dest[3] = \"abc\";\n"
" (void)strxfrm(dest,src,1);\n"
" (void)strxfrm(dest,src,2);\n"
" (void)strxfrm(dest,src,3);\n" // <<
"}");
ASSERT_EQUALS("[test.cpp:6]: (error) Buffer is accessed out of bounds: src\n", errout.str());
// source size is too small
check("void f(void) {\n"
" const char src[1] = \"a\";\n"
" char dest[3] = \"abc\";\n"
" (void)strxfrm(dest,src,1);\n"
" (void)strxfrm(dest,src,2);\n" // <<
"}");
ASSERT_EQUALS("[test.cpp:5]: (error) Buffer is accessed out of bounds: src\n", errout.str());
} }
void buffer_overrun_33() { // #2019 void buffer_overrun_33() { // #2019
@ -3403,6 +3387,32 @@ private:
ASSERT_EQUALS("", errout.str()); ASSERT_EQUALS("", errout.str());
} }
void buffer_overrun_37() { // #11765
check("void f() {\n"
" char buf[128];\n"
" memcpy(buf, \"Error\", 6);\n"
"}\n");
ASSERT_EQUALS("", errout.str());
check("void f() {\n"
" char buf[128];\n"
" memcpy(buf, \"Error\", 16);\n"
"}\n");
ASSERT_EQUALS("[test.cpp:3]: (error) Buffer is accessed out of bounds: \"Error\"\n", errout.str());
check("void f() {\n"
" char buf[128];\n"
" memcpy(buf, L\"Error\", 10);\n" // at least 12 bytes on any platform
"}\n");
ASSERT_EQUALS("", errout.str());
check("void f() {\n"
" char buf[128];\n"
" memcpy(buf, L\"Error\", 26);\n" // at most 24 bytes
"}\n");
ASSERT_EQUALS("[test.cpp:3]: (error) Buffer is accessed out of bounds: L\"Error\"\n", errout.str());
}
void buffer_overrun_errorpath() { void buffer_overrun_errorpath() {
setMultiline(); setMultiline();
const Settings settingsOld = settings0; const Settings settingsOld = settings0;
@ -4265,9 +4275,7 @@ private:
check("void f() {\n" check("void f() {\n"
" mymemset(\"abc\", 0, 20);\n" " mymemset(\"abc\", 0, 20);\n"
"}", settings); "}", settings);
TODO_ASSERT_EQUALS("[test.cpp:2]: (error) Buffer is accessed out of bounds.\n", ASSERT_EQUALS("[test.cpp:2]: (error) Buffer is accessed out of bounds: \"abc\"\n", errout.str());
"",
errout.str());
check("void f() {\n" check("void f() {\n"
" mymemset(temp, \"abc\", 4);\n" " mymemset(temp, \"abc\", 4);\n"