@@ -17,6 +17,7 @@ import codingstandards.c.cert
1717import codingstandards.cpp.Naming
1818import semmle.code.cpp.dataflow.TaintTracking
1919import codingstandards.cpp.PossiblyUnsafeStringOperation
20+ import semmle.code.cpp.valuenumbering.GlobalValueNumbering
2021
2122/**
2223 * Models a function that is part of the standard library that expects a
@@ -43,32 +44,90 @@ class ExpectsNullTerminatedStringAsArgumentFunctionCall extends FunctionCall {
4344 Expr getAnExpectingExpr ( ) { result = e }
4445}
4546
46- from ExpectsNullTerminatedStringAsArgumentFunctionCall fc , Expr e , Expr target
47- where
48- target = fc .getAnExpectingExpr ( ) and
49- not isExcluded ( fc , Strings1Package:: nonNullTerminatedToFunctionThatExpectsAStringQuery ( ) ) and
50- (
51- exists ( PossiblyUnsafeStringOperation op |
52- // don't report violations of the same function call.
53- not op = fc and
54- e = op and
55- TaintTracking:: localTaint ( DataFlow:: exprNode ( op .getAnArgument ( ) ) , DataFlow:: exprNode ( target ) )
47+ class PossiblyUnsafeStringOperationSource extends Source {
48+ PossiblyUnsafeStringOperation op ;
49+
50+ PossiblyUnsafeStringOperationSource ( ) { this .asExpr ( ) = op .getAnArgument ( ) }
51+
52+ PossiblyUnsafeStringOperation getOp ( ) { result = op }
53+ }
54+
55+ class CharArraySource extends Source {
56+ CharArrayInitializedWithStringLiteral op ;
57+
58+ CharArraySource ( ) {
59+ op .getContainerLength ( ) <= op .getStringLiteralLength ( ) and
60+ this .asExpr ( ) = op
61+ }
62+ }
63+
64+ abstract class Source extends DataFlow:: Node { }
65+
66+ class Sink extends DataFlow:: Node {
67+ Sink ( ) {
68+ exists ( ExpectsNullTerminatedStringAsArgumentFunctionCall fc |
69+ fc .getAnExpectingExpr ( ) = this .asExpr ( )
5670 )
57- or
58- exists ( CharArrayInitializedWithStringLiteral op |
59- e = op and
60- op .getContainerLength ( ) <= op .getStringLiteralLength ( ) and
61- TaintTracking:: localTaint ( DataFlow:: exprNode ( op ) , DataFlow:: exprNode ( target ) )
71+ }
72+ }
73+
74+ module MyFlowConfiguration implements DataFlow:: ConfigSig {
75+ predicate isSink ( DataFlow:: Node sink ) {
76+ sink instanceof Sink and
77+ //don't report violations of the same function call
78+ not sink instanceof Source
79+ }
80+
81+ predicate isSource ( DataFlow:: Node source ) { source instanceof Source }
82+
83+ predicate isAdditionalFlowStep ( DataFlow:: Node innode , DataFlow:: Node outnode ) {
84+ exists ( FunctionCall realloc , ReallocFunction fn |
85+ fn .getACallToThisFunction ( ) = realloc and
86+ realloc .getArgument ( 0 ) = innode .asExpr ( ) and
87+ realloc = outnode .asExpr ( )
6288 )
63- ) and
64- // don't report cases flowing to this node where there is a flow from a
65- // literal assignment of a null terminator
66- not exists ( AssignExpr aexp |
89+ }
90+ }
91+
92+ class ReallocFunction extends AllocationFunction {
93+ ReallocFunction ( ) { exists ( this .getReallocPtrArg ( ) ) }
94+ }
95+
96+ /**
97+ * Determines if the string is acceptably null terminated
98+ * The only condition we accept as a guarantee to null terminate is:
99+ * `str[size_expr] = '\0';`
100+ * where we do not check the value of the `size_expr` used
101+ */
102+ predicate isGuarded ( Expr guarded , Expr source ) {
103+ exists ( AssignExpr aexp |
67104 aexp .getLValue ( ) instanceof ArrayExpr and
68105 aexp .getRValue ( ) instanceof Zero and
69- TaintTracking:: localTaint ( DataFlow:: exprNode ( aexp .getRValue ( ) ) , DataFlow:: exprNode ( target ) ) and
70- // this must be AFTER the operation causing the non-null termination to be valid.
71- aexp .getAPredecessor * ( ) = e
106+ // this must be AFTER the operation causing the non-null termination
107+ aexp .getAPredecessor + ( ) = source and
108+ //this guards anything after it
109+ aexp .getASuccessor + ( ) = guarded and
110+ // no reallocs exist after this because they will be conservatively assumed to make the buffer smaller and remove the likliehood of this properly terminating
111+ not exists ( ReallocFunction realloc , FunctionCall fn |
112+ fn = realloc .getACallToThisFunction ( ) and
113+ globalValueNumber ( aexp .getLValue ( ) .( ArrayExpr ) .getArrayBase ( ) ) =
114+ globalValueNumber ( fn .getArgument ( 0 ) ) and
115+ aexp .getASuccessor + ( ) = fn
116+ )
72117 )
118+ }
119+
120+ module MyFlow = TaintTracking:: Global< MyFlowConfiguration > ;
121+
122+ from
123+ DataFlow:: Node source , DataFlow:: Node sink , ExpectsNullTerminatedStringAsArgumentFunctionCall fc ,
124+ Expr e
125+ where
126+ MyFlow:: flow ( source , sink ) and
127+ sink .asExpr ( ) = fc .getAnExpectingExpr ( ) and
128+ not isGuarded ( sink .asExpr ( ) , source .asExpr ( ) ) and
129+ if source instanceof PossiblyUnsafeStringOperationSource
130+ then e = source .( PossiblyUnsafeStringOperationSource ) .getOp ( )
131+ else e = source .asExpr ( )
73132select fc , "String modified by $@ is passed to function expecting a null-terminated string." , e ,
74133 "this expression"
0 commit comments