Skip to content

Commit e869452

Browse files
orbitcowboyorbitcowboy
committed
#10244: Fixed false negative: bufferAccessOutOfBounds
1 parent 44c8b31 commit e869452

File tree

3 files changed

+49
-10
lines changed

3 files changed

+49
-10
lines changed

‎lib/checkbufferoverrun.cpp‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -600,21 +600,21 @@ void CheckBufferOverrun::bufferOverflow()
600600
continue;
601601
// TODO: strcpy(buf+10, "hello");
602602
const ValueFlow::Value bufferSize = getBufferSize(argtok);
603-
if (bufferSize.intvalue <= 1)
603+
if (bufferSize.intvalue <= 0)
604604
continue;
605-
bool error = std::none_of(minsizes->begin(), minsizes->end(), [=](const Library::ArgumentChecks::MinSize &minsize) {
605+
const bool error = std::none_of(minsizes->begin(), minsizes->end(), [=](const Library::ArgumentChecks::MinSize &minsize) {
606606
return checkBufferSize(tok, minsize, args, bufferSize.intvalue, mSettings);
607607
});
608608
if (error)
609-
bufferOverflowError(args[argnr], &bufferSize);
609+
bufferOverflowError(args[argnr], &bufferSize, (bufferSize.intvalue == 1) ? Certainty::inconclusive : Certainty::normal);
610610
}
611611
}
612612
}
613613
}
614614

615-
void CheckBufferOverrun::bufferOverflowError(const Token *tok, const ValueFlow::Value *value)
615+
void CheckBufferOverrun::bufferOverflowError(const Token *tok, const ValueFlow::Value *value, const Certainty::CertaintyLevel &certainty)
616616
{
617-
reportError(getErrorPath(tok, value, "Buffer overrun"), Severity::error, "bufferAccessOutOfBounds", "Buffer is accessed out of bounds: " + (tok ? tok->expressionString() : "buf"), CWE_BUFFER_OVERRUN, Certainty::normal);
617+
reportError(getErrorPath(tok, value, "Buffer overrun"), Severity::error, "bufferAccessOutOfBounds", "Buffer is accessed out of bounds: " + (tok ? tok->expressionString() : "buf"), CWE_BUFFER_OVERRUN, certainty);
618618
}
619619

620620
//---------------------------------------------------------------------------

‎lib/checkbufferoverrun.h‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
#include "check.h"
2626
#include "config.h"
2727
#include "ctu.h"
28+
#include "errortypes.h"
2829
#include "mathlib.h"
2930
#include "symboldatabase.h"
3031
#include "valueflow.h"
@@ -83,7 +84,7 @@ class CPPCHECKLIB CheckBufferOverrun : public Check {
8384
c.pointerArithmeticError(nullptr, nullptr, nullptr);
8485
c.negativeIndexError(nullptr, std::vector<Dimension>(), std::vector<const ValueFlow::Value *>());
8586
c.arrayIndexThenCheckError(nullptr, "i");
86-
c.bufferOverflowError(nullptr, nullptr);
87+
c.bufferOverflowError(nullptr, nullptr, Certainty::normal);
8788
c.objectIndexError(nullptr, nullptr, true);
8889
}
8990

@@ -103,7 +104,7 @@ class CPPCHECKLIB CheckBufferOverrun : public Check {
103104
void pointerArithmeticError(const Token *tok, const Token *indexToken, const ValueFlow::Value *indexValue);
104105

105106
void bufferOverflow();
106-
void bufferOverflowError(const Token *tok, const ValueFlow::Value *value);
107+
void bufferOverflowError(const Token *tok, const ValueFlow::Value *value, const Certainty::CertaintyLevel& certainty);
107108

108109
void arrayIndexThenCheck();
109110
void arrayIndexThenCheckError(const Token *tok, const std::string &indexName);

‎test/testbufferoverrun.cpp‎

Lines changed: 41 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,7 @@ class TestBufferOverrun : public TestFixture {
180180
TEST_CASE(buffer_overrun_29); // #7083: false positive: typedef and initialization with strings
181181
TEST_CASE(buffer_overrun_30); // #6367
182182
TEST_CASE(buffer_overrun_31);
183+
TEST_CASE(buffer_overrun_32); //#10244
183184
TEST_CASE(buffer_overrun_errorpath);
184185
TEST_CASE(buffer_overrun_bailoutIfSwitch); // ticket #2378 : bailoutIfSwitch
185186
// TODO TEST_CASE(buffer_overrun_function_array_argument);
@@ -2652,6 +2653,43 @@ class TestBufferOverrun : public TestFixture {
26522653
ASSERT_EQUALS("", errout.str());
26532654
}
26542655

2656+
void buffer_overrun_32() {
2657+
// destination size is too small
2658+
check("void f(void) {\n"
2659+
" const char src[3] = \"abc\";\n"
2660+
" char dest[1] = \"a\";\n"
2661+
" (void)strxfrm(dest,src,1);\n"
2662+
" (void)strxfrm(dest,src,2);\n"// <<
2663+
"}");
2664+
ASSERT_EQUALS("[test.cpp:5]: (error, inconclusive) Buffer is accessed out of bounds: dest\n", errout.str());
2665+
// destination size is too small
2666+
check("void f(void) {\n"
2667+
" const char src[3] = \"abc\";\n"
2668+
" char dest[2] = \"ab\";\n"
2669+
" (void)strxfrm(dest,src,1);\n"
2670+
" (void)strxfrm(dest,src,2);\n"
2671+
" (void)strxfrm(dest,src,3);\n" // <<
2672+
"}");
2673+
ASSERT_EQUALS("[test.cpp:6]: (error) Buffer is accessed out of bounds: dest\n", errout.str());
2674+
// source size is too small
2675+
check("void f(void) {\n"
2676+
" const char src[2] = \"ab\";\n"
2677+
" char dest[3] = \"abc\";\n"
2678+
" (void)strxfrm(dest,src,1);\n"
2679+
" (void)strxfrm(dest,src,2);\n"
2680+
" (void)strxfrm(dest,src,3);\n" // <<
2681+
"}");
2682+
ASSERT_EQUALS("[test.cpp:6]: (error) Buffer is accessed out of bounds: src\n", errout.str());
2683+
// source size is too small
2684+
check("void f(void) {\n"
2685+
" const char src[1] = \"a\";\n"
2686+
" char dest[3] = \"abc\";\n"
2687+
" (void)strxfrm(dest,src,1);\n"
2688+
" (void)strxfrm(dest,src,2);\n" // <<
2689+
"}");
2690+
ASSERT_EQUALS("[test.cpp:5]: (error, inconclusive) Buffer is accessed out of bounds: src\n", errout.str());
2691+
}
2692+
26552693
void buffer_overrun_errorpath() {
26562694
setMultiline();
26572695
settings0.templateLocation = "{file}:{line}:note:{info}";
@@ -3609,7 +3647,7 @@ class TestBufferOverrun : public TestFixture {
36093647
" struct Foo x;\n"
36103648
" mysprintf(x.a, \"aa\");\n"
36113649
"}", settings);
3612-
TODO_ASSERT_EQUALS("[test.cpp:4]: (error) Buffer is accessed out of bounds: x.a\n", "", errout.str());
3650+
ASSERT_EQUALS("[test.cpp:4]: (error, inconclusive) Buffer is accessed out of bounds: x.a\n", errout.str());
36133651

36143652
// ticket #900
36153653
check("void f() {\n"
@@ -3630,14 +3668,14 @@ class TestBufferOverrun : public TestFixture {
36303668
" struct Foo *x = malloc(sizeof(Foo));\n"
36313669
" mysprintf(x.a, \"aa\");\n"
36323670
"}", settings);
3633-
ASSERT_EQUALS("", errout.str()); // TODO: Inconclusive error?
3671+
ASSERT_EQUALS("[test.cpp:4]: (error, inconclusive) Buffer is accessed out of bounds: x.a\n", errout.str());
36343672

36353673
check("struct Foo { char a[1]; };\n"
36363674
"void f() {\n"
36373675
" struct Foo *x = malloc(sizeof(Foo) + 10);\n"
36383676
" mysprintf(x.a, \"aa\");\n"
36393677
"}", settings);
3640-
ASSERT_EQUALS("", errout.str());
3678+
ASSERT_EQUALS("[test.cpp:4]: (error, inconclusive) Buffer is accessed out of bounds: x.a\n", errout.str());
36413679

36423680
check("struct Foo {\n" // #6668 - unknown size
36433681
" char a[LEN];\n"

0 commit comments

Comments
 (0)