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

Buffer overflow: Add CTU checking for pointer arithmetic overflows

This commit is contained in:
Daniel Marjamäki 2019-04-03 06:43:56 +02:00
parent 9f3ecdde31
commit de4f57ec0f
4 changed files with 120 additions and 61 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
lib/analyzerinfo.cpp
View File

@ -53,14 +53,14 @@ void AnalyzerInformation::writeFilesTxt(const std::string &buildDir, const std::
const std::string afile = getFilename(f);
if (fileCount.find(afile) == fileCount.end())
fileCount[afile] = 0;
fout << afile << ".a" << (++fileCount[afile]) << "::" << Path::fromNativeSeparators(f) << '\n';
fout << afile << ".a" << (++fileCount[afile]) << "::" << Path::simplifyPath(Path::fromNativeSeparators(f)) << '\n';
}
for (const ImportProject::FileSettings &fs : fileSettings) {
const std::string afile = getFilename(fs.filename);
if (fileCount.find(afile) == fileCount.end())
fileCount[afile] = 0;
fout << afile << ".a" << (++fileCount[afile]) << ":" << fs.cfg << ":" << Path::fromNativeSeparators(fs.filename) << std::endl;
fout << afile << ".a" << (++fileCount[afile]) << ":" << fs.cfg << ":" << Path::simplifyPath(Path::fromNativeSeparators(fs.filename)) << std::endl;
}
}

142
lib/checkbufferoverrun.cpp
View File

@ -740,49 +740,80 @@ void CheckBufferOverrun::bufferNotZeroTerminatedError(const Token *tok, const st
std::string CheckBufferOverrun::MyFileInfo::toString() const
{
return CTU::toString(unsafeUsage);
std::string xml;
if (!unsafeArrayIndex.empty())
xml = " <array-index>\n" + CTU::toString(unsafeArrayIndex) + " </array-index>\n";
if (!unsafePointerArith.empty())
xml += " <pointer-arith>\n" + CTU::toString(unsafePointerArith) + " </pointer-arith>\n";
return xml;
}
bool CheckBufferOverrun::isCtuUnsafeBufferUsage(const Check *check, const Token *argtok, MathLib::bigint *offset)
bool CheckBufferOverrun::isCtuUnsafeBufferUsage(const Check *check, const Token *argtok, MathLib::bigint *offset, int type)
{
const CheckBufferOverrun *c = dynamic_cast<const CheckBufferOverrun *>(check);
if (!c)
return false;
if (!argtok->valueType())
return false;
if (!Token::Match(argtok, "%name% [") || argtok->astParent() != argtok->next() || Token::simpleMatch(argtok->linkAt(1), "] ["))
const Token *indexTok = nullptr;
if (type == 1 && Token::Match(argtok, "%name% [") && argtok->astParent() == argtok->next() && !Token::simpleMatch(argtok->linkAt(1), "] ["))
indexTok = argtok->next()->astOperand2();
else if (type == 2 && Token::simpleMatch(argtok->astParent(), "+"))
indexTok = (argtok == argtok->astParent()->astOperand1()) ?
argtok->astParent()->astOperand2() :
argtok->astParent()->astOperand1();
if (!indexTok)
return false;
if (!argtok->next()->astOperand2())
return false;
if (!argtok->next()->astOperand2()->hasKnownIntValue())
if (!indexTok->hasKnownIntValue())
return false;
if (!offset)
return false;
*offset = argtok->next()->astOperand2()->getKnownIntValue() * argtok->valueType()->typeSize(*c->mSettings);
*offset = indexTok->getKnownIntValue() * argtok->valueType()->typeSize(*c->mSettings);
return true;
}
bool CheckBufferOverrun::isCtuUnsafeArrayIndex(const Check *check, const Token *argtok, MathLib::bigint *offset)
{
return CheckBufferOverrun::isCtuUnsafeBufferUsage(check, argtok, offset, 1);
}
bool CheckBufferOverrun::isCtuUnsafePointerArith(const Check *check, const Token *argtok, MathLib::bigint *offset)
{
return CheckBufferOverrun::isCtuUnsafeBufferUsage(check, argtok, offset, 2);
}
/** @brief Parse current TU and extract file info */
Check::FileInfo *CheckBufferOverrun::getFileInfo(const Tokenizer *tokenizer, const Settings *settings) const
{
CheckBufferOverrun checkBufferOverrun(tokenizer, settings, nullptr);
const std::list<CTU::FileInfo::UnsafeUsage> &unsafeUsage = CTU::getUnsafeUsage(tokenizer, settings, &checkBufferOverrun, isCtuUnsafeBufferUsage);
if (unsafeUsage.empty())
return nullptr;
MyFileInfo *fileInfo = new MyFileInfo;
fileInfo->unsafeUsage = unsafeUsage;
fileInfo->unsafeArrayIndex = CTU::getUnsafeUsage(tokenizer, settings, &checkBufferOverrun, isCtuUnsafeArrayIndex);
fileInfo->unsafePointerArith = CTU::getUnsafeUsage(tokenizer, settings, &checkBufferOverrun, isCtuUnsafePointerArith);
if (fileInfo->unsafeArrayIndex.empty() && fileInfo->unsafePointerArith.empty()) {
delete fileInfo;
return nullptr;
}
return fileInfo;
}
Check::FileInfo * CheckBufferOverrun::loadFileInfoFromXml(const tinyxml2::XMLElement *xmlElement) const
{
const std::list<CTU::FileInfo::UnsafeUsage> &unsafeUsage = CTU::loadUnsafeUsageListFromXml(xmlElement);
if (unsafeUsage.empty())
return nullptr;
const std::string arrayIndex("array-index");
const std::string pointerArith("pointer-arith");
MyFileInfo *fileInfo = new MyFileInfo;
fileInfo->unsafeUsage = unsafeUsage;
for (const tinyxml2::XMLElement *e = xmlElement->FirstChildElement(); e; e = e->NextSiblingElement()) {
if (e->Name() == arrayIndex)
fileInfo->unsafeArrayIndex = CTU::loadUnsafeUsageListFromXml(e);
else if (e->Name() == pointerArith)
fileInfo->unsafePointerArith = CTU::loadUnsafeUsageListFromXml(e);
}
if (fileInfo->unsafeArrayIndex.empty() && fileInfo->unsafePointerArith.empty()) {
delete fileInfo;
return nullptr;
}
return fileInfo;
}
@ -800,40 +831,53 @@ bool CheckBufferOverrun::analyseWholeProgram(const CTU::FileInfo *ctu, const std
const MyFileInfo *fi = dynamic_cast<MyFileInfo*>(fi1);
if (!fi)
continue;
for (const CTU::FileInfo::UnsafeUsage &unsafeUsage : fi->unsafeUsage) {
const CTU::FileInfo::FunctionCall *functionCall = nullptr;
const std::list<ErrorLogger::ErrorMessage::FileLocation> &locationList =
ctu->getErrorPath(CTU::FileInfo::InvalidValueType::bufferOverflow,
unsafeUsage,
callsMap,
"Using argument ARG",
&functionCall,
false);
if (locationList.empty())
continue;
if (unsafeUsage.value > 0) {
const ErrorLogger::ErrorMessage errmsg(locationList,
emptyString,
Severity::error,
"Buffer access out of bounds; '" + unsafeUsage.myArgumentName + "' buffer size is " + MathLib::toString(functionCall->callArgValue) + " and it is accessed at offset " + MathLib::toString(unsafeUsage.value) + ".",
"ctubufferoverrun",
CWE_BUFFER_OVERRUN, false);
errorLogger.reportErr(errmsg);
} else {
const ErrorLogger::ErrorMessage errmsg(locationList,
emptyString,
Severity::error,
"Buffer access out of bounds; buffer '" + unsafeUsage.myArgumentName + "' is accessed at offset " + MathLib::toString(unsafeUsage.value) + ".",
"ctubufferunderrun",
CWE_BUFFER_UNDERRUN, false);
errorLogger.reportErr(errmsg);
}
foundErrors = true;
}
for (const CTU::FileInfo::UnsafeUsage &unsafeUsage : fi->unsafeArrayIndex)
foundErrors |= analyseWholeProgram1(ctu, callsMap, unsafeUsage, 1, errorLogger);
for (const CTU::FileInfo::UnsafeUsage &unsafeUsage : fi->unsafePointerArith)
foundErrors |= analyseWholeProgram1(ctu, callsMap, unsafeUsage, 2, errorLogger);
}
return foundErrors;
}
bool CheckBufferOverrun::analyseWholeProgram1(const CTU::FileInfo *ctu, const std::map<std::string, std::list<const CTU::FileInfo::CallBase *>> &callsMap, const CTU::FileInfo::UnsafeUsage &unsafeUsage, int type, ErrorLogger &errorLogger)
{
const CTU::FileInfo::FunctionCall *functionCall = nullptr;
const std::list<ErrorLogger::ErrorMessage::FileLocation> &locationList =
ctu->getErrorPath(CTU::FileInfo::InvalidValueType::bufferOverflow,
unsafeUsage,
callsMap,
"Using argument ARG",
&functionCall,
false);
if (locationList.empty())
return false;
const char *errorId = nullptr;
std::string errmsg;
CWE cwe(0);
if (type == 1) {
errorId = "ctuArrayIndex";
if (unsafeUsage.value > 0)
errmsg = "Array index out of bounds; '" + unsafeUsage.myArgumentName + "' buffer size is " + MathLib::toString(functionCall->callArgValue) + " and it is accessed at offset " + MathLib::toString(unsafeUsage.value) + ".";
else
errmsg = "Array index out of bounds; buffer '" + unsafeUsage.myArgumentName + "' is accessed at offset " + MathLib::toString(unsafeUsage.value) + ".";
cwe = (unsafeUsage.value > 0) ? CWE_BUFFER_OVERRUN : CWE_BUFFER_UNDERRUN;
} else {
errorId = "ctuPointerArith";
errmsg = "Pointer arithmetic overflow; '" + unsafeUsage.myArgumentName + "' buffer size is " + MathLib::toString(functionCall->callArgValue);
cwe = CWE_POINTER_ARITHMETIC_OVERFLOW;
}
const ErrorLogger::ErrorMessage errorMessage(locationList,
emptyString,
Severity::error,
errmsg,
errorId,
cwe, false);
errorLogger.reportErr(errorMessage);
return true;
}

13
lib/checkbufferoverrun.h
View File

@ -85,7 +85,6 @@ public:
/** @brief Analyse all file infos for all TU */
bool analyseWholeProgram(const CTU::FileInfo *ctu, const std::list<Check::FileInfo*> &fileInfo, const Settings& settings, ErrorLogger &errorLogger) OVERRIDE;
private:
void arrayIndex();
@ -112,16 +111,22 @@ private:
/** data for multifile checking */
class MyFileInfo : public Check::FileInfo {
public:
/** function arguments that data are unconditionally read */
std::list<CTU::FileInfo::UnsafeUsage> unsafeUsage;
/** unsafe array index usage */
std::list<CTU::FileInfo::UnsafeUsage> unsafeArrayIndex;
/** unsafe pointer arithmetics */
std::list<CTU::FileInfo::UnsafeUsage> unsafePointerArith;
/** Convert MyFileInfo data into xml string */
std::string toString() const OVERRIDE;
};
static bool isCtuUnsafeBufferUsage(const Check *check, const Token *argtok, MathLib::bigint *value);
static bool isCtuUnsafeBufferUsage(const Check *check, const Token *argtok, MathLib::bigint *value, int type);
static bool isCtuUnsafeArrayIndex(const Check *check, const Token *argtok, MathLib::bigint *value);
static bool isCtuUnsafePointerArith(const Check *check, const Token *argtok, MathLib::bigint *value);
Check::FileInfo * loadFileInfoFromXml(const tinyxml2::XMLElement *xmlElement) const OVERRIDE;
bool analyseWholeProgram1(const CTU::FileInfo *ctu, const std::map<std::string, std::list<const CTU::FileInfo::CallBase *>> &callsMap, const CTU::FileInfo::UnsafeUsage &unsafeUsage, int type, ErrorLogger &errorLogger);
static std::string myName() {

22
test/testbufferoverrun.cpp
View File

@ -249,6 +249,7 @@ private:
TEST_CASE(ctu_malloc);
TEST_CASE(ctu_array);
TEST_CASE(ctu_variable);
TEST_CASE(ctu_arithmetic);
}
@ -4088,7 +4089,7 @@ private:
" char *s = malloc(4);\n"
" dostuff(s);\n"
"}");
ASSERT_EQUALS("[test.cpp:6] -> [test.cpp:7] -> [test.cpp:2]: (error) Buffer access out of bounds; buffer 'p' is accessed at offset -3.\n", errout.str());
ASSERT_EQUALS("[test.cpp:6] -> [test.cpp:7] -> [test.cpp:2]: (error) Array index out of bounds; buffer 'p' is accessed at offset -3.\n", errout.str());
ctu("void dostuff(char *p) {\n"
" p[4] = 0;\n"
@ -4098,7 +4099,7 @@ private:
" char *s = malloc(4);\n"
" dostuff(s);\n"
"}");
ASSERT_EQUALS("[test.cpp:6] -> [test.cpp:7] -> [test.cpp:2]: (error) Buffer access out of bounds; 'p' buffer size is 4 and it is accessed at offset 4.\n", errout.str());
ASSERT_EQUALS("[test.cpp:6] -> [test.cpp:7] -> [test.cpp:2]: (error) Array index out of bounds; 'p' buffer size is 4 and it is accessed at offset 4.\n", errout.str());
}
void ctu_array() {
@ -4109,7 +4110,7 @@ private:
" char str[4];\n"
" dostuff(str);\n"
"}");
ASSERT_EQUALS("[test.cpp:6] -> [test.cpp:2]: (error) Buffer access out of bounds; 'p' buffer size is 4 and it is accessed at offset 10.\n", errout.str());
ASSERT_EQUALS("[test.cpp:6] -> [test.cpp:2]: (error) Array index out of bounds; 'p' buffer size is 4 and it is accessed at offset 10.\n", errout.str());
ctu("static void memclr( char *data )\n"
"{\n"
@ -4121,7 +4122,7 @@ private:
" char str[5];\n"
" memclr( str );\n"
"}");
ASSERT_EQUALS("[test.cpp:9] -> [test.cpp:3]: (error) Buffer access out of bounds; 'data' buffer size is 5 and it is accessed at offset 10.\n", errout.str());
ASSERT_EQUALS("[test.cpp:9] -> [test.cpp:3]: (error) Array index out of bounds; 'data' buffer size is 5 and it is accessed at offset 10.\n", errout.str());
ctu("static void memclr( int i, char *data )\n"
"{\n"
@ -4133,7 +4134,7 @@ private:
" char str[5];\n"
" memclr( 0, str );\n"
"}");
ASSERT_EQUALS("[test.cpp:9] -> [test.cpp:3]: (error) Buffer access out of bounds; 'data' buffer size is 5 and it is accessed at offset 10.\n", errout.str());
ASSERT_EQUALS("[test.cpp:9] -> [test.cpp:3]: (error) Array index out of bounds; 'data' buffer size is 5 and it is accessed at offset 10.\n", errout.str());
ctu("static void memclr( int i, char *data )\n"
"{\n"
@ -4185,7 +4186,16 @@ private:
" int x = 4;\n"
" dostuff(&x);\n"
"}");
ASSERT_EQUALS("[test.cpp:6] -> [test.cpp:2]: (error) Buffer access out of bounds; 'p' buffer size is 4 and it is accessed at offset 40.\n", errout.str());
ASSERT_EQUALS("[test.cpp:6] -> [test.cpp:2]: (error) Array index out of bounds; 'p' buffer size is 4 and it is accessed at offset 40.\n", errout.str());
}
void ctu_arithmetic() {
ctu("void dostuff(int *p) { x = p + 10; }\n"
"int main() {\n"
" int x[3];\n"
" dostuff(x);\n"
"}");
ASSERT_EQUALS("[test.cpp:4] -> [test.cpp:1]: (error) Pointer arithmetic overflow; 'p' buffer size is 12\n", errout.str());
}
};