Skip to content

CPP: Add query for CWE-754: Improper Check for Unusual or Exceptional Conditions when using functions scanf #8246

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.

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
merged 22 commits into from
Mar 11, 2022
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
...
r = scanf("%i", &i);
if (r == 1) // GOOD
return i;
else
return -1;
...
scanf("%i", &i); // BAD
return i;
...
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
<!DOCTYPE qhelp PUBLIC
"-//Semmle//qhelp//EN"
"qhelp.dtd">
<qhelp>
<overview>
<p>Working with reading data without validation procedures and with uninitialized arguments can lead to unpredictable consequences.</p>

</recommendation>
<example>
<p>The following example demonstrates erroneous and corrected work with a function call.</p>
<sample src="ImproperCheckReturnValueScanf.cpp" />

</example>
<references>

<li>
CERT C Coding Standard:
<a href="https://wiki.sei.cmu.edu/confluence/display/c/EXP12-C.+Do+not+ignore+values+returned+by+functions">EXP12-C. Do not ignore values returned by functions</a>.
</li>

</references>
</qhelp>
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
/**
* @name Improper check return value scanf
* @description Using a function call without the ability to evaluate the correctness of the work can lead to unexpected results.
* @kind problem
* @id cpp/improper-check-return-value-scanf
* @problem.severity warning
* @precision medium
* @tags correctness
* security
* external/cwe/cwe-754
* external/cwe/cwe-908
*/

import cpp
import semmle.code.cpp.commons.Exclusions

/** Returns the position of the first argument being filled. */
int posArgumentInFunctionCall(FunctionCall fc) {
(
fc.getTarget().hasGlobalOrStdName(["scanf", "scanf_s"]) and
result = 1
or
fc.getTarget().hasGlobalOrStdName(["fscanf", "sscanf", "fscanf_s", "sscanf_s"]) and
result = 2
)
}

/** Holds if a function argument was not initialized but used after the call. */
predicate argumentIsNotInitializedAndIsUsed(Variable vt, FunctionCall fc) {
exists(Expr e0 |
// Fillable argument was not initialized.
vt instanceof LocalScopeVariable and
not vt.getAnAssignment().getASuccessor+() = fc and
(
not vt.hasInitializer()
or
exists(Expr e1, Variable v1 |
e1 = vt.getInitializer().getExpr() and
v1 = e1.(AddressOfExpr).getOperand().(VariableAccess).getTarget() and
(
not v1.hasInitializer() and
not v1.getAnAssignment().getASuccessor+() = fc
)
)
) and
not exists(AssignExpr ae |
ae.getLValue() = vt.getAnAccess().getParent() and
ae.getASuccessor+() = fc
) and
not exists(FunctionCall f0 |
f0.getAnArgument().getAChild() = vt.getAnAccess() and
f0.getASuccessor+() = fc
) and
// After the call, the completed arguments are assigned or returned as the result of the operation of the upper function.
fc.getASuccessor+() = e0 and
(
(
e0.(Assignment).getRValue().(VariableAccess).getTarget() = vt or
e0.(Assignment).getRValue().(ArrayExpr).getArrayBase().(VariableAccess).getTarget() = vt
)
or
e0.getEnclosingStmt() instanceof ReturnStmt and
e0.(VariableAccess).getTarget() = vt
or
not exists(Expr e1 |
fc.getASuccessor+() = e1 and
e1.(VariableAccess).getTarget() = vt
)
)
)
}

from FunctionCall fc, int i
where
// Function return value is not evaluated.
fc instanceof ExprInVoidContext and
not isFromMacroDefinition(fc) and
i in [posArgumentInFunctionCall(fc) .. fc.getNumberOfArguments() - 1] and
(
argumentIsNotInitializedAndIsUsed(fc.getArgument(i).(VariableAccess).getTarget(), fc) or
argumentIsNotInitializedAndIsUsed(fc.getArgument(i)
.(AddressOfExpr)
.getOperand()
.(VariableAccess)
.getTarget(), fc) or
argumentIsNotInitializedAndIsUsed(fc.getArgument(i)
.(ArrayExpr)
.getArrayBase()
.(VariableAccess)
.getTarget(), fc)
) and
// After the call, filled arguments are not evaluated.
not exists(Expr e0, int i1 |
i1 in [posArgumentInFunctionCall(fc) .. fc.getNumberOfArguments() - 1] and
fc.getASuccessor+() = e0 and
e0.getEnclosingElement() instanceof ComparisonOperation and
(
e0.(VariableAccess).getTarget() = fc.getArgument(i1).(VariableAccess).getTarget() or
e0.(VariableAccess).getTarget() =
fc.getArgument(i1).(AddressOfExpr).getOperand().(VariableAccess).getTarget()
)
)
select fc, "Unchecked return value for call to '" + fc.getTarget().getName() + "'."
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
| test.cpp:51:3:51:7 | call to scanf | Unchecked return value for call to 'scanf'. |
| test.cpp:52:3:52:7 | call to scanf | Unchecked return value for call to 'scanf'. |
| test.cpp:53:3:53:7 | call to scanf | Unchecked return value for call to 'scanf'. |
| test.cpp:104:3:104:7 | call to scanf | Unchecked return value for call to 'scanf'. |
| test.cpp:105:3:105:7 | call to scanf | Unchecked return value for call to 'scanf'. |
| test.cpp:106:3:106:7 | call to scanf | Unchecked return value for call to 'scanf'. |
| test.cpp:114:3:114:7 | call to scanf | Unchecked return value for call to 'scanf'. |
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
experimental/Security/CWE/CWE-754/ImproperCheckReturnValueScanf.ql
Original file line number Diff line number Diff line change
@@ -0,0 +1,117 @@
int scanf(const char *format, ...);
int globalVal;
char * globalVala;
int * globalValp;
char globalVala2;
int functionWork1(int retIndex) {
int i;
char a[10];
int b;
int *p = &b;
if (scanf("%i", &i) != 1) // GOOD
return -1;
if (scanf("%s", a) != 1) // GOOD
return -1;
if (scanf("%i", p) != 1) // GOOD
return -1;
if(retIndex == 0)
return (int)*a;
if(retIndex == 1)
return *p;
return i;
}

int functionWork1_(int retIndex) {
int i;
char a[10];
int b;
int *p = &b;
int r;
r = scanf("%i", &i);
if (r != 1) // GOOD
return -1;
r = scanf("%s", a);
if (r == 1) // GOOD
return -1;
r = scanf("%i", p);
if (r != 1) // GOOD
return -1;
if(retIndex == 0)
return (int)*a;
if(retIndex == 1)
return *p;
return i;
}

int functionWork1b(int retIndex) {
int i;
char a[10];
int b;
int *p = &b;
scanf("%i", &i); // BAD
scanf("%s", a); // BAD
scanf("%i", p); // BAD
if(retIndex == 0)
return (int)*a;
if(retIndex == 1)
return *p;
return i;
}
int functionWork1_() {
int i;
scanf("%i",&i); // GOOD
if(i<10)
return -1;
return i;
}
int functionWork2(int retIndex) {
int i = 0;
char a[10] = "";
int b = 1;
int *p = &b;
scanf("%i", &i); // GOOD:Argument initialized even when scanf fails.
scanf("%s", a); // GOOD:Argument initialized even when scanf fails.
scanf("%i", p); // GOOD:Argument initialized even when scanf fails.
if(retIndex == 0)
return (int)*a;
if(retIndex == 1)
return *p;
return i;
}

int functionWork2_(int retIndex) {
int i;
i = 0;
char a[10];
a[0] = '\0';
int b;
b=1;
int *p = &b;
scanf("%i", &i); // GOOD:Argument initialized even when scanf fails.
scanf("%s", a); // GOOD:Argument initialized even when scanf fails.
scanf("%i", p); // GOOD:Argument initialized even when scanf fails.
if(retIndex == 0)
return (int)*a;
if(retIndex == 1)
return *p;
return i;
}
int functionWork2b() {
int i;
char a[10];
int b;
int *p = &b;
scanf("%i", &i); // BAD
scanf("%s", a); // BAD
scanf("%i", p); // BAD
globalVal = i;
globalVala = a;
globalValp = p;
return 0;
}
int functionWork2b_() {
char a[10];
scanf("%s", a); // BAD
globalVala2 = a[0];
return 0;
}