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

Fixed #6349 (Pointer arithmetic: clarify message)

This commit is contained in:
Daniel Marjamäki 2014-12-25 10:05:55 +01:00
parent 26aa049724
commit bc594d52c8
3 changed files with 32 additions and 21 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

41
lib/checkbufferoverrun.cpp
View File

@ -173,13 +173,24 @@ void CheckBufferOverrun::outOfBoundsError(const Token *tok, const std::string &w
reportError(tok, Severity::error, "outOfBounds", oss.str()); reportError(tok, Severity::error, "outOfBounds", oss.str());
} }
void CheckBufferOverrun::pointerOutOfBoundsError(const Token *tok, const std::string &object) void CheckBufferOverrun::pointerOutOfBoundsError(const Token *tok, const Token *index, const MathLib::bigint indexvalue)
{ {
// The severity is portability instead of error since this ub doesnt // The severity is portability instead of error since this ub doesnt
// cause bad behaviour on most implementations. people create out // cause bad behaviour on most implementations. people create out
// of bounds pointers by intention. // of bounds pointers by intention.
reportError(tok, Severity::portability, "pointerOutOfBounds", "Undefined behaviour: Pointer arithmetic result does not point into or just past the end of the " + object + ".\n" const std::string expr(tok ? tok->expressionString() : std::string(""));
"Undefined behaviour: The result of this pointer arithmetic does not point into or just one element past the end of the " + object + ". Further information: https://www.securecoding.cert.org/confluence/display/seccode/ARR30-C.+Do+not+form+or+use+out+of+bounds+pointers+or+array+subscripts"); std::string errmsg("Undefined behaviour. Pointer arithmetic '" + expr + "' result is out of bounds");
if (index && !index->isNumber())
errmsg += " when " + index->expressionString() + " is " + MathLib::toString(indexvalue);
std::string verbosemsg(errmsg + ". From chapter 6.5.6 in the C specification:\n"
"\"When an expression that has integer type is added to or subtracted from a pointer, ..\" and then \"If both the pointer operand and the result point to elements of the same array object, or one past the last element of the array object, the evaluation shall not produce an overflow; otherwise, the behavior is undefined.\"");
reportError(tok, Severity::portability, "pointerOutOfBounds", errmsg + ".\n" + verbosemsg);
/*
"Undefined behaviour: The result of this pointer arithmetic does not point into
or just one element past the end of the " + object + ".
Further information:
https://www.securecoding.cert.org/confluence/display/seccode/ARR30-C.+Do+not+form+or+use+out+of+bounds+pointers+or+array+subscripts");
*/
} }
void CheckBufferOverrun::sizeArgumentAsCharError(const Token *tok) void CheckBufferOverrun::sizeArgumentAsCharError(const Token *tok)
@ -699,7 +710,7 @@ void CheckBufferOverrun::checkScope(const Token *tok, const std::vector<std::str
else if (declarationId && Token::Match(tok, "= %varid% + %num% ;", declarationId)) { else if (declarationId && Token::Match(tok, "= %varid% + %num% ;", declarationId)) {
const MathLib::bigint index = MathLib::toLongNumber(tok->strAt(3)); const MathLib::bigint index = MathLib::toLongNumber(tok->strAt(3));
if (isPortabilityEnabled && index > size) if (isPortabilityEnabled && index > size)
pointerOutOfBoundsError(tok->next(), "buffer"); pointerOutOfBoundsError(tok->tokAt(2));
if (index >= size && Token::Match(tok->tokAt(-2), "[;{}] %varid% =", declarationId)) if (index >= size && Token::Match(tok->tokAt(-2), "[;{}] %varid% =", declarationId))
pointerIsOutOfBounds = true; pointerIsOutOfBounds = true;
} }
@ -851,24 +862,24 @@ void CheckBufferOverrun::checkScope(const Token *tok, const ArrayInfo &arrayInfo
} }
else if (isPortabilityEnabled && !tok->isCast() && tok->astParent() && tok->astParent()->str() == "+") { else if (isPortabilityEnabled && !tok->isCast() && tok->astParent() && tok->astParent()->str() == "+") {
const ValueFlow::Value *index;
if (tok == tok->astParent()->astOperand1())
index = tok->astParent()->astOperand2()->getMaxValue(false);
else
index = tok->astParent()->astOperand1()->getMaxValue(false);
// undefined behaviour: result of pointer arithmetic is out of bounds // undefined behaviour: result of pointer arithmetic is out of bounds
if (index && (index->intvalue < 0 || index->intvalue > arrayInfo.num(0))) { const Token *index;
pointerOutOfBoundsError(tok, "array"); if (tok == tok->astParent()->astOperand1())
} index = tok->astParent()->astOperand2();
else
index = tok->astParent()->astOperand1();
const ValueFlow::Value *value = index ? index->getMaxValue(false) : nullptr;
if (value && (value->intvalue < 0 || value->intvalue > arrayInfo.num(0)))
pointerOutOfBoundsError(tok->astParent(), index, value->intvalue);
} }
else if (isPortabilityEnabled && tok->astParent() && tok->astParent()->str() == "-") { else if (isPortabilityEnabled && tok->astParent() && tok->astParent()->str() == "-") {
const Variable *var = _tokenizer->getSymbolDatabase()->getVariableFromVarId(declarationId); const Variable *var = _tokenizer->getSymbolDatabase()->getVariableFromVarId(declarationId);
if (var && var->isArray()) { if (var && var->isArray()) {
const Token *index = tok->astParent()->astOperand2(); const Token *index = tok->astParent()->astOperand2();
if (index && index->getValueGE(1,_settings)) const ValueFlow::Value *value = index ? index->getValueGE(1,_settings) : nullptr;
pointerOutOfBoundsError(tok, "array"); if (value)
pointerOutOfBoundsError(tok->astParent(), index, value->intvalue);
} }
} }
} }

4
lib/checkbufferoverrun.h
View File

@ -240,7 +240,7 @@ private:
void negativeIndexError(const Token *tok, MathLib::bigint index); void negativeIndexError(const Token *tok, MathLib::bigint index);
void negativeIndexError(const Token *tok, const ValueFlow::Value &index); void negativeIndexError(const Token *tok, const ValueFlow::Value &index);
void cmdLineArgsError(const Token *tok); void cmdLineArgsError(const Token *tok);
void pointerOutOfBoundsError(const Token *tok, const std::string &object); // UB when result of calculation is out of bounds void pointerOutOfBoundsError(const Token *tok, const Token *index=nullptr, const MathLib::bigint indexvalue=0);
void arrayIndexThenCheckError(const Token *tok, const std::string &indexName); void arrayIndexThenCheckError(const Token *tok, const std::string &indexName);
void possibleBufferOverrunError(const Token *tok, const std::string &src, const std::string &dst, bool cat); void possibleBufferOverrunError(const Token *tok, const std::string &src, const std::string &dst, bool cat);
void possibleReadlinkBufferOverrunError(const Token *tok, const std::string &funcname, const std::string &varname); void possibleReadlinkBufferOverrunError(const Token *tok, const std::string &funcname, const std::string &varname);
@ -263,7 +263,7 @@ public:
c.bufferNotZeroTerminatedError(0, "buffer", "strncpy"); c.bufferNotZeroTerminatedError(0, "buffer", "strncpy");
c.negativeIndexError(0, -1); c.negativeIndexError(0, -1);
c.cmdLineArgsError(0); c.cmdLineArgsError(0);
c.pointerOutOfBoundsError(0, "array"); c.pointerOutOfBoundsError(nullptr, nullptr, 0);
c.arrayIndexThenCheckError(0, "index"); c.arrayIndexThenCheckError(0, "index");
c.possibleBufferOverrunError(0, "source", "destination", false); c.possibleBufferOverrunError(0, "source", "destination", false);
c.possibleReadlinkBufferOverrunError(0, "readlink", "buffer"); c.possibleReadlinkBufferOverrunError(0, "readlink", "buffer");

8
test/testbufferoverrun.cpp
View File

@ -2962,13 +2962,13 @@ private:
" char a[10];\n" " char a[10];\n"
" char *p = a + 100;\n" " char *p = a + 100;\n"
"}"); "}");
ASSERT_EQUALS("[test.cpp:3]: (portability) Undefined behaviour: Pointer arithmetic result does not point into or just past the end of the array.\n", errout.str()); ASSERT_EQUALS("[test.cpp:3]: (portability) Undefined behaviour. Pointer arithmetic 'a+100' result is out of bounds.\n", errout.str());
check("void f() {\n" check("void f() {\n"
" char a[10];\n" " char a[10];\n"
" return a + 100;\n" " return a + 100;\n"
"}"); "}");
ASSERT_EQUALS("[test.cpp:3]: (portability) Undefined behaviour: Pointer arithmetic result does not point into or just past the end of the array.\n", errout.str()); ASSERT_EQUALS("[test.cpp:3]: (portability) Undefined behaviour. Pointer arithmetic 'a+100' result is out of bounds.\n", errout.str());
check("void f() {\n" // #6350 - fp when there is cast of buffer check("void f() {\n" // #6350 - fp when there is cast of buffer
" wchar_t buf[64];\n" " wchar_t buf[64];\n"
@ -2983,7 +2983,7 @@ private:
" p += 100;\n" " p += 100;\n"
" free(p);" " free(p);"
"}"); "}");
ASSERT_EQUALS("[test.cpp:3]: (portability) Undefined behaviour: Pointer arithmetic result does not point into or just past the end of the buffer.\n", errout.str()); ASSERT_EQUALS("[test.cpp:3]: (portability) Undefined behaviour. Pointer arithmetic 'p+100' result is out of bounds.\n", errout.str());
check("void f() {\n" check("void f() {\n"
" char *p = malloc(10);\n" " char *p = malloc(10);\n"
@ -3017,7 +3017,7 @@ private:
" char x[10];\n" " char x[10];\n"
" return x-1;\n" " return x-1;\n"
"}"); "}");
ASSERT_EQUALS("[test.cpp:3]: (portability) Undefined behaviour: Pointer arithmetic result does not point into or just past the end of the array.\n", errout.str()); ASSERT_EQUALS("[test.cpp:3]: (portability) Undefined behaviour. Pointer arithmetic 'x-1' result is out of bounds.\n", errout.str());
} }
void sprintf1() { void sprintf1() {