Skip to content

Fix #901 Improve out-of-bounds check to detect error with sprintf() #8473

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
chrchr-github merged 2 commits into from
Apr 30, 2026
Merged

Fix #901 Improve out-of-bounds check to detect error with sprintf() #8473

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
473c3fd
Fix #901 Improve out-of-bounds check to detect error with sprintf()
web-flow Apr 21, 2026
6a29bf8
Update testbufferoverrun.cpp
chrchr-github Apr 27, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -509,7 +509,7 @@ $(libcppdir)/checkautovariables.o: lib/checkautovariables.cpp lib/addoninfo.h li
$(libcppdir)/checkbool.o: lib/checkbool.cpp lib/addoninfo.h lib/astutils.h lib/check.h lib/checkbool.h lib/checkers.h lib/config.h lib/errortypes.h lib/library.h lib/mathlib.h lib/platform.h lib/regex.h lib/settings.h lib/smallvector.h lib/sourcelocation.h lib/standards.h lib/symboldatabase.h lib/templatesimplifier.h lib/token.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/vfvalue.h
$(CXX) ${INCLUDE_FOR_LIB} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ $(libcppdir)/checkbool.cpp

$(libcppdir)/checkbufferoverrun.o: lib/checkbufferoverrun.cpp externals/tinyxml2/tinyxml2.h lib/addoninfo.h lib/astutils.h lib/check.h lib/checkbufferoverrun.h lib/checkers.h lib/config.h lib/ctu.h lib/errorlogger.h lib/errortypes.h lib/library.h lib/mathlib.h lib/path.h lib/platform.h lib/regex.h lib/settings.h lib/smallvector.h lib/sourcelocation.h lib/standards.h lib/symboldatabase.h lib/templatesimplifier.h lib/token.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/valueflow.h lib/vfvalue.h lib/xml.h
$(libcppdir)/checkbufferoverrun.o: lib/checkbufferoverrun.cpp externals/tinyxml2/tinyxml2.h lib/addoninfo.h lib/astutils.h lib/check.h lib/checkbufferoverrun.h lib/checkers.h lib/config.h lib/ctu.h lib/errorlogger.h lib/errortypes.h lib/library.h lib/mathlib.h lib/path.h lib/platform.h lib/regex.h lib/settings.h lib/smallvector.h lib/sourcelocation.h lib/standards.h lib/symboldatabase.h lib/templatesimplifier.h lib/token.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/valueflow.h lib/vf_common.h lib/vfvalue.h lib/xml.h
$(CXX) ${INCLUDE_FOR_LIB} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ $(libcppdir)/checkbufferoverrun.cpp

$(libcppdir)/checkclass.o: lib/checkclass.cpp externals/tinyxml2/tinyxml2.h lib/addoninfo.h lib/astutils.h lib/check.h lib/checkclass.h lib/checkers.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/library.h lib/mathlib.h lib/path.h lib/platform.h lib/regex.h lib/settings.h lib/smallvector.h lib/sourcelocation.h lib/standards.h lib/symboldatabase.h lib/templatesimplifier.h lib/token.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/vfvalue.h lib/xml.h
Expand Down
9 changes: 5 additions & 4 deletions lib/checkbufferoverrun.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
#include "utils.h"
#include "valueflow.h"
#include "vfvalue.h"
#include "vf_common.h"

#include <algorithm>
#include <cstdlib>
Expand Down Expand Up @@ -83,7 +84,7 @@
return (op->valueType() && op->valueType()->pointer) ? op : tok;
}

static int getMinFormatStringOutputLength(const std::vector<const Token*> &parameters, nonneg int formatStringArgNr)
static int getMinFormatStringOutputLength(const std::vector<const Token*> &parameters, nonneg int formatStringArgNr, const Settings& settings)
{
if (formatStringArgNr <= 0 || formatStringArgNr > parameters.size())
return 0;
Expand Down Expand Up @@ -138,8 +139,8 @@
break;
case 's':
parameterLength = 0;
if (inputArgNr < parameters.size() && parameters[inputArgNr]->tokType() == Token::eString)
parameterLength = Token::getStrLength(parameters[inputArgNr]);
if (inputArgNr < parameters.size())
parameterLength = ValueFlow::valueFlowGetStrLength(parameters[inputArgNr], settings);

Check failure

Code scanning / Cppcheck Premium

Guarantee that container indices and iterators are within the valid range Critical

Guarantee that container indices and iterators are within the valid range
Comment thread
github-advanced-security[bot] marked this conversation as resolved.
Fixed
Comment thread
chrchr-github marked this conversation as resolved.

handleNextParameter = true;
break;
Expand Down Expand Up @@ -602,7 +603,7 @@
switch (minsize.type) {
case Library::ArgumentChecks::MinSize::Type::STRLEN:
if (settings.library.isargformatstr(ftok, minsize.arg)) {
return getMinFormatStringOutputLength(args, minsize.arg) < bufferSize;
return getMinFormatStringOutputLength(args, minsize.arg, settings) < bufferSize;
} else if (arg) {
const Token *strtoken = arg->getValueTokenMaxStrLength();
if (strtoken)
Expand Down
6 changes: 3 additions & 3 deletions lib/valueflow.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1843,9 +1843,9 @@
const bool isCpp = (src && src->isCpp()) || (dst && dst->isCpp());
if (isNotEqual(decl, parentdecl) && !(isCpp && (Token::simpleMatch(decl.first, "auto") || Token::simpleMatch(parentdecl.first, "auto"))))
return true;
if (isNotEqual(decl, dst->valueType(), isCpp, settings))

Check failure on line 1846 in lib/valueflow.cpp

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Called C++ object pointer is null 


See more on https://sonarcloud.io/project/issues?id=danmar_cppcheck&issues=AZ2xOEHPEKMbOcCSm09r&open=AZ2xOEHPEKMbOcCSm09r&pullRequest=8473
return true;
if (isNotEqual(parentdecl, src->valueType(), isCpp, settings))

Check failure on line 1848 in lib/valueflow.cpp

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Called C++ object pointer is null 


See more on https://sonarcloud.io/project/issues?id=danmar_cppcheck&issues=AZ2xOEHPEKMbOcCSm09s&open=AZ2xOEHPEKMbOcCSm09s&pullRequest=8473
return true;
}
return false;
Expand Down Expand Up @@ -6803,17 +6803,17 @@
} else if (tok->str() == "+=" && astIsContainer(tok->astOperand1())) {
const Token* containerTok = tok->astOperand1();
const Token* valueTok = tok->astOperand2();
const MathLib::bigint size = ValueFlow::valueFlowGetStrLength(valueTok);
const MathLib::bigint size = ValueFlow::valueFlowGetStrLength(valueTok, settings);
forwardMinimumContainerSize(size, tok, containerTok);

} else if (tok->str() == "=" && Token::simpleMatch(tok->astOperand2(), "+") && astIsContainerString(tok)) {
const Token* tok2 = tok->astOperand2();
MathLib::bigint size = 0;
while (Token::simpleMatch(tok2, "+") && tok2->astOperand2()) {
size += ValueFlow::valueFlowGetStrLength(tok2->astOperand2());
size += ValueFlow::valueFlowGetStrLength(tok2->astOperand2(), settings);
tok2 = tok2->astOperand1();
}
size += ValueFlow::valueFlowGetStrLength(tok2);
size += ValueFlow::valueFlowGetStrLength(tok2, settings);
forwardMinimumContainerSize(size, tok, tok->astOperand1());
}
}
Expand Down
2 changes: 1 addition & 1 deletion lib/vf_analyzers.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1547,7 +1547,7 @@ struct ContainerExpressionAnalyzer : ExpressionAnalyzer {
case Library::Container::Action::APPEND: {
std::vector<const Token*> args = getArguments(tok->astParent()->tokAt(2));
if (args.size() == 1) // TODO: handle overloads
n = ValueFlow::valueFlowGetStrLength(tok->astParent()->tokAt(3));
n = ValueFlow::valueFlowGetStrLength(tok->astParent()->tokAt(3), settings);
if (n == 0) // TODO: handle known empty append
val->setPossible();
break;
Expand Down
6 changes: 4 additions & 2 deletions lib/vf_common.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -384,7 +384,7 @@ namespace ValueFlow
v.debugPath.emplace_back(tok, std::move(s));
}

MathLib::bigint valueFlowGetStrLength(const Token* tok)
MathLib::bigint valueFlowGetStrLength(const Token* tok, const Settings& settings)
Comment thread
chrchr-github marked this conversation as resolved.
{
if (tok->tokType() == Token::eString)
return Token::getStrLength(tok);
Expand All @@ -394,8 +394,10 @@ namespace ValueFlow
return v->intvalue;
if (const Value* v = tok->getKnownValue(Value::ValueType::TOK)) {
if (v->tokvalue != tok)
return valueFlowGetStrLength(v->tokvalue);
return valueFlowGetStrLength(v->tokvalue, settings);
}
if (const Token* cont = settings.library.getContainerFromYield(tok, Library::Container::Yield::BUFFER_NT))
return valueFlowGetStrLength(cont, settings);
return 0;
}
}
2 changes: 1 addition & 1 deletion lib/vf_common.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ namespace ValueFlow
const Token* tok,
SourceLocation local = SourceLocation::current());

MathLib::bigint valueFlowGetStrLength(const Token* tok);
MathLib::bigint valueFlowGetStrLength(const Token* tok, const Settings& settings);
}

#endif // vfCommonH
2 changes: 1 addition & 1 deletion oss-fuzz/Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ $(libcppdir)/checkautovariables.o: ../lib/checkautovariables.cpp ../lib/addoninf
$(libcppdir)/checkbool.o: ../lib/checkbool.cpp ../lib/addoninfo.h ../lib/astutils.h ../lib/check.h ../lib/checkbool.h ../lib/checkers.h ../lib/config.h ../lib/errortypes.h ../lib/library.h ../lib/mathlib.h ../lib/platform.h ../lib/regex.h ../lib/settings.h ../lib/smallvector.h ../lib/sourcelocation.h ../lib/standards.h ../lib/symboldatabase.h ../lib/templatesimplifier.h ../lib/token.h ../lib/tokenize.h ../lib/tokenlist.h ../lib/utils.h ../lib/vfvalue.h
$(CXX) ${LIB_FUZZING_ENGINE} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ $(libcppdir)/checkbool.cpp

$(libcppdir)/checkbufferoverrun.o: ../lib/checkbufferoverrun.cpp ../externals/tinyxml2/tinyxml2.h ../lib/addoninfo.h ../lib/astutils.h ../lib/check.h ../lib/checkbufferoverrun.h ../lib/checkers.h ../lib/config.h ../lib/ctu.h ../lib/errorlogger.h ../lib/errortypes.h ../lib/library.h ../lib/mathlib.h ../lib/path.h ../lib/platform.h ../lib/regex.h ../lib/settings.h ../lib/smallvector.h ../lib/sourcelocation.h ../lib/standards.h ../lib/symboldatabase.h ../lib/templatesimplifier.h ../lib/token.h ../lib/tokenize.h ../lib/tokenlist.h ../lib/utils.h ../lib/valueflow.h ../lib/vfvalue.h ../lib/xml.h
$(libcppdir)/checkbufferoverrun.o: ../lib/checkbufferoverrun.cpp ../externals/tinyxml2/tinyxml2.h ../lib/addoninfo.h ../lib/astutils.h ../lib/check.h ../lib/checkbufferoverrun.h ../lib/checkers.h ../lib/config.h ../lib/ctu.h ../lib/errorlogger.h ../lib/errortypes.h ../lib/library.h ../lib/mathlib.h ../lib/path.h ../lib/platform.h ../lib/regex.h ../lib/settings.h ../lib/smallvector.h ../lib/sourcelocation.h ../lib/standards.h ../lib/symboldatabase.h ../lib/templatesimplifier.h ../lib/token.h ../lib/tokenize.h ../lib/tokenlist.h ../lib/utils.h ../lib/valueflow.h ../lib/vf_common.h ../lib/vfvalue.h ../lib/xml.h
$(CXX) ${LIB_FUZZING_ENGINE} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ $(libcppdir)/checkbufferoverrun.cpp

$(libcppdir)/checkclass.o: ../lib/checkclass.cpp ../externals/tinyxml2/tinyxml2.h ../lib/addoninfo.h ../lib/astutils.h ../lib/check.h ../lib/checkclass.h ../lib/checkers.h ../lib/config.h ../lib/errorlogger.h ../lib/errortypes.h ../lib/library.h ../lib/mathlib.h ../lib/path.h ../lib/platform.h ../lib/regex.h ../lib/settings.h ../lib/smallvector.h ../lib/sourcelocation.h ../lib/standards.h ../lib/symboldatabase.h ../lib/templatesimplifier.h ../lib/token.h ../lib/tokenize.h ../lib/tokenlist.h ../lib/utils.h ../lib/vfvalue.h ../lib/xml.h
Expand Down
20 changes: 20 additions & 0 deletions test/testbufferoverrun.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4610,6 +4610,26 @@ class TestBufferOverrun : public TestFixture {
" mysprintf(a, \"abcd\");\n"
"}", settings);
ASSERT_EQUALS("", errout_str());

check("void f() {\n" // #901
Comment thread
chrchr-github marked this conversation as resolved.
" const char b[] = \"b\";\n"
" char a[1];\n"
" sprintf(a, \"%s\", b);\n"
"}\n"
"void g() {\n"
" const char* b = \"b\";\n"
" char a[1];\n"
" sprintf(a, \"%s\", b);\n"
"}\n"
"void h() {\n"
" const std::string b = \"b\";\n"
" char a[1];\n"
" sprintf(a, \"%s\", b.c_str());\n"
"}", settings0);
ASSERT_EQUALS("[test.cpp:4:13]: (error) Buffer is accessed out of bounds: a [bufferAccessOutOfBounds]\n"
"[test.cpp:9:13]: (error) Buffer is accessed out of bounds: a [bufferAccessOutOfBounds]\n"
"[test.cpp:14:13]: (error) Buffer is accessed out of bounds: a [bufferAccessOutOfBounds]\n",
errout_str());
}

void minsize_mul() {
Expand Down
Loading