From 3f587bef653f9d3176f212057768699e4c217871 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Sat, 28 Sep 2019 19:28:12 +0200 Subject: [PATCH] ExprEngine: Add some CWE476 (Null pointer dereference) checks --- lib/exprengine.cpp | 58 ++++++++++++++++++++++++++++++++++++++----- test/verify/juliet.py | 56 +++++++++++++++++++++++++++-------------- 2 files changed, 89 insertions(+), 25 deletions(-) diff --git a/lib/exprengine.cpp b/lib/exprengine.cpp index b5ea525f3..22f5733bd 100644 --- a/lib/exprengine.cpp +++ b/lib/exprengine.cpp @@ -300,6 +300,15 @@ static ExprEngine::ValuePtr translateUninitValueToRange(ExprEngine::ValuePtr val return value; } +static int128_t truncateInt(int128_t value, int bits, char sign) +{ + value = value & (((int128_t)1 << bits) - 1); + // Sign extension + if (sign == 's' && value & (1ULL << (bits - 1))) + value |= ~(((int128_t)1 << bits) - 1); + return value; +} + ExprEngine::ArrayValue::ArrayValue(const std::string &name, ExprEngine::ValuePtr size, ExprEngine::ValuePtr value) : Value(name, ExprEngine::ValueType::ArrayValue) , size(size) @@ -641,6 +650,14 @@ ExprEngine::BinOpResult::IntOrFloatValue ExprEngine::BinOpResult::evaluateOperan result.setFloatValue(valueType ? floatRange->minValue : floatRange->maxValue); return result; } + if (auto integerTruncation = std::dynamic_pointer_cast(value)) { + ExprEngine::BinOpResult::IntOrFloatValue result(evaluateOperand(test, valueBit, integerTruncation->inputValue)); + if (result.isFloat()) + result.setIntValue(truncateInt(result.floatValue, integerTruncation->bits, integerTruncation->sign)); + else + result.setIntValue(truncateInt(result.intValue, integerTruncation->bits, integerTruncation->sign)); + return result; + } throw std::runtime_error("Internal error: Unhandled value:" + std::to_string((int)value->type)); } @@ -730,11 +747,7 @@ static ExprEngine::ValuePtr truncateValue(ExprEngine::ValuePtr val, const ValueT if (auto range = std::dynamic_pointer_cast(val)) { if (range->minValue == range->maxValue) { - int128_t newValue = range->minValue; - newValue = newValue & (((int128_t)1 << bits) - 1); - // Sign extension - if (valueType->sign == ValueType::Sign::SIGNED && newValue & (1ULL << (bits - 1))) - newValue |= ~(((int128_t)1 << bits) - 1); + int128_t newValue = truncateInt(range->minValue, bits, valueType->sign == ValueType::Sign::SIGNED ? 's' : 'u'); if (newValue == range->minValue) return val; return std::make_shared(ExprEngine::str(newValue), newValue, newValue); @@ -856,6 +869,11 @@ static ExprEngine::ValuePtr executeArrayIndex(const Token *tok, Data &data) call(data.callbacks, tok, value.second); return std::make_shared(data.getNewSymbolName(), conditionalValues); } + + // TODO: Pointer value.. + executeExpression(tok->astOperand1(), data); + executeExpression(tok->astOperand2(), data); + return ExprEngine::ValuePtr(); } @@ -1164,7 +1182,7 @@ void ExprEngine::executeFunction(const Scope *functionScope, const Tokenizer *to void ExprEngine::runChecks(ErrorLogger *errorLogger, const Tokenizer *tokenizer, const Settings *settings) { - std::function divByZero = [&](const Token *tok, const ExprEngine::Value &value) { + std::function divByZero = [=](const Token *tok, const ExprEngine::Value &value) { if (!Token::Match(tok->astParent(), "[/%]")) return; if (tok->astParent()->astOperand2() == tok && value.isIntValueInRange(0)) { @@ -1174,6 +1192,33 @@ void ExprEngine::runChecks(ErrorLogger *errorLogger, const Tokenizer *tokenizer, } }; + std::function nullPointerDereference = [=](const Token *tok, const ExprEngine::Value &value) { + if (!tok->astParent()) + return; + + // Is pointer dereferenced? + bool deref = false; + deref |= tok->astParent()->isUnaryOp("*"); + deref |= Token::simpleMatch(tok->astParent(), "["); + if (!deref) + return; + + // Is this a null pointer value? + try { + if (auto pointerValue = dynamic_cast(&value)) { + if (!pointerValue->null) + return; + } else if (!value.isIntValueInRange(0)) + return; + } catch (const std::exception &) { + return; + } + + std::list callstack{tok->astParent()}; + ErrorLogger::ErrorMessage errmsg(callstack, &tokenizer->list, Severity::SeverityType::error, "verificationNullPointerDereference", "There is pointer dereference, cannot determine that the pointer can't be NULL.", CWE(476), false); + errorLogger->reportErr(errmsg); + }; + std::function integerOverflow = [&](const Token *tok, const ExprEngine::Value &value) { if (!tok->isArithmeticalOp() || !tok->valueType() || !tok->valueType()->isIntegral() || tok->valueType()->pointer > 0) return; @@ -1212,6 +1257,7 @@ void ExprEngine::runChecks(ErrorLogger *errorLogger, const Tokenizer *tokenizer, std::vector callbacks; callbacks.push_back(divByZero); + callbacks.push_back(nullPointerDereference); #ifdef VERIFY_INTEGEROVERFLOW callbacks.push_back(integerOverflow); #endif diff --git a/test/verify/juliet.py b/test/verify/juliet.py index 70bca7b12..a3b73e02c 100644 --- a/test/verify/juliet.py +++ b/test/verify/juliet.py @@ -10,10 +10,9 @@ import subprocess JULIET_PATH = os.path.expanduser('~/juliet') CPPCHECK_PATH = '../../cppcheck' -def get_files(juliet_path:str): +def get_files(juliet_path:str, test_cases:str): ret = [] - pattern = 'C/testcases/CWE369_Divide_by_Zero/s01/CWE369_Divide_by_Zero__int_*divide*.c' - g = os.path.join(juliet_path, pattern) + g = os.path.join(juliet_path, test_cases) print(g) for f in sorted(glob.glob(g)): res = re.match(r'(.*[0-9][0-9])[a-x]?.(cp*)$', f) @@ -25,23 +24,42 @@ def get_files(juliet_path:str): ret.append(f) return ret -num_ok = 0 -num_failed = 0 -for f in get_files(JULIET_PATH): - inc = '-I' + os.path.join(JULIET_PATH, 'C/testcasesupport') - cmd = f'{CPPCHECK_PATH} {inc} -DOMIT_GOOD --library=posix --verify --platform=unix64 ' + ' '.join(glob.glob(f)) - p = subprocess.Popen(cmd.split(), stdout=subprocess.PIPE, stderr=subprocess.PIPE) - comm = p.communicate() - stdout = comm[0].decode(encoding='utf-8', errors='ignore') - stderr = comm[1].decode(encoding='utf-8', errors='ignore') +def check(tc:str, warning_id:str): + num_ok = 0 + num_failed = 0 - if 'verificationDivByZero' in stderr: - num_ok += 1 - else: - print(f'fail: {cmd}') - #print(stdout) - num_failed += 1 + for f in get_files(JULIET_PATH, tc): + cmd = [CPPCHECK_PATH, + '-I' + os.path.join(JULIET_PATH, 'C/testcasesupport'), + '-DOMIT_GOOD', + '--library=posix', + '--verify', + '--platform=unix64'] + cmd += glob.glob(f) + p = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE) + comm = p.communicate() + stdout = comm[0].decode(encoding='utf-8', errors='ignore') + stderr = comm[1].decode(encoding='utf-8', errors='ignore') -print(f'ok:{num_ok}, fail:{num_failed}') + if warning_id in stderr: + num_ok += 1 + else: + print(f'fail: ' + ' '.join(cmd)) + num_failed += 1 + + cwepos = tc.find('CWE') + cwe = tc[cwepos:cwepos+6] + + return f'{cwe} ok:{num_ok}, fail:{num_failed}\n' + + +final_report = '' +final_report += check('C/testcases/CWE369_Divide_by_Zero/s*/*_int_*.c', 'verificationDivByZero') +final_report += check('C/testcases/CWE476_*/*.c', 'verificationNullPointerDereference') + +print(final_report) + +assert final_report == ('CWE369 ok:456, fail:0\n' + 'CWE476 ok:186, fail:84\n')