mirror of
https://github.com/github/codeql.git
synced 2026-02-23 18:33:42 +01:00
C++: Refactor leap year logic for UncheckedLeapYearAfterYearModification. Includes new logic for detecting leap year checks, new forms of leap year checks detected, and various heuristics to remove false postives. Move TimeConversionFunction into LeapYear.qll and refactored to separate conversion functions that are expected to be checked for failure from those that auto correct leap year dates if feb 29 is provided on a non-leap year. Increas the set of known TimeConversionFunctions.
This commit is contained in:
REDMOND\brodes
@@ -308,3 +308,35 @@ private module PossibleYearArithmeticOperationCheckConfig implements DataFlow::C
|
||||
|
||||
module PossibleYearArithmeticOperationCheckFlow =
|
||||
TaintTracking::Global<PossibleYearArithmeticOperationCheckConfig>;
|
||||
|
||||
/**
|
||||
* This list of APIs should check for the return value to detect problems during the conversion.
|
||||
*/
|
||||
class TimeConversionFunction extends Function {
|
||||
boolean autoLeapYearCorrecting;
|
||||
|
||||
TimeConversionFunction() {
|
||||
autoLeapYearCorrecting = false and
|
||||
(
|
||||
this.getName() =
|
||||
[
|
||||
"FileTimeToSystemTime", "SystemTimeToFileTime", "SystemTimeToTzSpecificLocalTime",
|
||||
"SystemTimeToTzSpecificLocalTimeEx", "TzSpecificLocalTimeToSystemTime",
|
||||
"TzSpecificLocalTimeToSystemTimeEx", "RtlLocalTimeToSystemTime",
|
||||
"RtlTimeToSecondsSince1970", "_mkgmtime", "SetSystemTime", "VarUdateFromDate", "from_tm"
|
||||
]
|
||||
or
|
||||
// Matches all forms of GetDateFormat, e.g. GetDateFormatA/W/Ex
|
||||
this.getName().matches("GetDateFormat%")
|
||||
)
|
||||
or
|
||||
autoLeapYearCorrecting = true and
|
||||
this.getName() =
|
||||
["mktime", "_mktime32", "_mktime64", "SystemTimeToVariantTime", "VariantTimeToSystemTime"]
|
||||
}
|
||||
|
||||
/**
|
||||
* Holds if the function is expected to auto convert a bad leap year date.
|
||||
*/
|
||||
predicate isAutoLeapYearCorrecting() { autoLeapYearCorrecting = true }
|
||||
}
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
/**
|
||||
* @name Year field changed using an arithmetic operation without checking for leap year
|
||||
* @description A field that represents a year is being modified by an arithmetic operation, but no proper check for leap years can be detected afterwards.
|
||||
* @kind problem
|
||||
* @kind path-problem
|
||||
* @problem.severity warning
|
||||
* @id cpp/leap-year/unchecked-after-arithmetic-year-modification
|
||||
* @precision medium
|
||||
@@ -11,49 +11,832 @@
|
||||
|
||||
import cpp
|
||||
import LeapYear
|
||||
import semmle.code.cpp.controlflow.IRGuards
|
||||
import semmle.code.cpp.ir.IR
|
||||
import semmle.code.cpp.commons.DateTime
|
||||
|
||||
from Variable var, LeapYearFieldAccess yfa
|
||||
where
|
||||
exists(VariableAccess va |
|
||||
yfa.getQualifier() = va and
|
||||
var.getAnAccess() = va and
|
||||
// The year is modified with an arithmetic operation. Avoid values that are likely false positives
|
||||
yfa.isModifiedByArithmeticOperationNotForNormalization() and
|
||||
// Avoid false positives
|
||||
not (
|
||||
// If there is a local check for leap year after the modification
|
||||
exists(LeapYearFieldAccess yfacheck |
|
||||
yfacheck.getQualifier() = var.getAnAccess() and
|
||||
yfacheck.isUsedInCorrectLeapYearCheck() and
|
||||
yfacheck.getBasicBlock() = yfa.getBasicBlock().getASuccessor*()
|
||||
)
|
||||
/**
|
||||
* Functions whose operations should never be considered a
|
||||
* source or sink of a dangerous leap year operation.
|
||||
* The general concept is to add conversion functions
|
||||
* that convert one time type to another. Often
|
||||
* other ignorable operation heuristics will filter these,
|
||||
* but some cases, the simplest approach is to simply filter
|
||||
* the function entirely.
|
||||
* Note that flow through these functions should still be allowed
|
||||
* we just cannot start or end flow from an operation to a
|
||||
* year assignment in one of these functions.
|
||||
*/
|
||||
class IgnorableFunction extends Function {
|
||||
IgnorableFunction() {
|
||||
this instanceof TimeConversionFunction
|
||||
or
|
||||
// Helper utility in postgres with string time conversions
|
||||
this.getName() = "DecodeISO8601Interval"
|
||||
or
|
||||
// helper utility for date conversions in qtbase
|
||||
this.getName() = "adjacentDay"
|
||||
or
|
||||
// Windows API function that does timezone conversions
|
||||
this.getName().matches("%SystemTimeToTzSpecificLocalTime%")
|
||||
or
|
||||
// Windows APIs that do time conversions
|
||||
this.getName().matches("%localtime%\\_s%")
|
||||
or
|
||||
// Windows APIs that do time conversions
|
||||
this.getName().matches("%SpecificLocalTimeToSystemTime%")
|
||||
or
|
||||
// postgres function for diffing timestamps, date for leap year
|
||||
// is not applicable.
|
||||
this.getName().toLowerCase().matches("%timestamp%age%")
|
||||
or
|
||||
// Reading byte streams often involves operations of some base, but that's
|
||||
// not a real source of leap year issues.
|
||||
this.getName().toLowerCase().matches("%read%bytes%")
|
||||
or
|
||||
// A postgres function for local time conversions
|
||||
// conversion operations (from one time structure to another) are generally ignorable
|
||||
this.getName() = "localsub"
|
||||
or
|
||||
// Indication of a calendar not applicable to
|
||||
// gregorian leap year, e.g., Hijri, Persian, Hebrew
|
||||
this.getName().toLowerCase().matches("%hijri%")
|
||||
or
|
||||
this.getFile().getBaseName().toLowerCase().matches("%hijri%")
|
||||
or
|
||||
this.getName().toLowerCase().matches("%persian%")
|
||||
or
|
||||
this.getFile().getBaseName().toLowerCase().matches("%persian%")
|
||||
or
|
||||
this.getName().toLowerCase().matches("%hebrew%")
|
||||
or
|
||||
this.getFile().getBaseName().toLowerCase().matches("%hebrew%")
|
||||
or
|
||||
// misc. from string/char converters heuristic
|
||||
this.getName()
|
||||
.toLowerCase()
|
||||
.matches(["%char%to%", "%string%to%", "%from%char%", "%from%string%"])
|
||||
or
|
||||
// boost's gregorian.cpp has year manipulations that are checked in complex ways.
|
||||
// ignore the entire file as a source or sink.
|
||||
this.getFile().getAbsolutePath().toLowerCase().matches("%boost%gregorian.cpp%")
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* The set of expressions which are ignorable; either because they seem to not be part of a year mutation,
|
||||
* or because they seem to be a conversion pattern of mapping date scalars.
|
||||
*/
|
||||
abstract class IgnorableOperation extends Expr { }
|
||||
|
||||
class IgnorableExprRem extends IgnorableOperation instanceof RemExpr { }
|
||||
|
||||
/**
|
||||
* Anything involving an operation with 10, 100, 1000, 10000 is often a sign of conversion
|
||||
* or atoi.
|
||||
*/
|
||||
class IgnorableExpr10MulipleComponent extends IgnorableOperation {
|
||||
IgnorableExpr10MulipleComponent() {
|
||||
this.(Operation).getAnOperand().getValue().toInt() in [10, 100, 1000, 10000]
|
||||
or
|
||||
exists(AssignOperation a | a.getRValue() = this |
|
||||
a.getRValue().getValue().toInt() in [10, 100, 1000, 10000]
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Anything involving a sub expression with char literal 48, ignore as a likely string conversion
|
||||
* e.g., X - '0'
|
||||
*/
|
||||
class IgnorableExpr48Mapping extends IgnorableOperation {
|
||||
IgnorableExpr48Mapping() {
|
||||
this.(SubExpr).getRightOperand().getValue().toInt() = 48
|
||||
or
|
||||
exists(AssignSubExpr e | e.getRValue() = this | e.getRValue().getValue().toInt() = 48)
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* A binary or arithemtic operation whereby one of the components is textual or a string.
|
||||
*/
|
||||
class IgnorableCharLiteralArithmetic extends IgnorableOperation {
|
||||
IgnorableCharLiteralArithmetic() {
|
||||
this.(BinaryArithmeticOperation).getAnOperand() instanceof TextLiteral
|
||||
or
|
||||
this instanceof TextLiteral and
|
||||
any(AssignArithmeticOperation arith).getRValue() = this
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Constants often used in date conversions (from one date data type to another)
|
||||
* Numerous examples exist, like 1900 or 2000 that convert years from one
|
||||
* representation to another.
|
||||
* Also '0' is sometimes observed as an atoi style conversion.
|
||||
*/
|
||||
bindingset[c]
|
||||
predicate isLikelyConversionConstant(int c) {
|
||||
exists(int i | i = c.abs() |
|
||||
i =
|
||||
[
|
||||
146097, // days in 400-year Gregorian cycle
|
||||
36524, // days in 100-year Gregorian subcycle
|
||||
1461, // days in 4-year cycle (incl. 1 leap)
|
||||
32044, // Fliegel–van Flandern JDN epoch shift
|
||||
1721425, // JDN of 0001‑01‑01 (Gregorian)
|
||||
1721119, // alt epoch offset
|
||||
2400000, // MJD → JDN conversion
|
||||
2400001, // alt MJD → JDN conversion
|
||||
2141, // fixed‑point month/day extraction
|
||||
65536, // observed in some conversions
|
||||
7834, // observed in some conversions
|
||||
256, // observed in some conversions
|
||||
292275056, // qdatetime.h Qt Core year range first year constant
|
||||
292278994, // qdatetime.h Qt Core year range last year constant
|
||||
1601, // Windows FILETIME epoch start year
|
||||
1970, // Unix epoch start year
|
||||
70, // Unix epoch start year short form
|
||||
1899, // Observed in uses with 1900 to address off by one scenarios
|
||||
1900, // Used when converting a 2 digit year
|
||||
2000, // Used when converting a 2 digit year
|
||||
1400, // Hijri base year, used when converting a 2 digit year
|
||||
1980, // FAT filesystem epoch start year
|
||||
227013, // constant observed for Hirji year conversion, and Hirji years are not applicable for gregorian leap year
|
||||
10631, // constant observed for Hirji year conversion, and Hirji years are not applicable for gregorian leap year
|
||||
0
|
||||
]
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
* Some constants indicate conversion that are ignorable, e.g.,
|
||||
* julian to gregorian conversion or conversions from linux time structs
|
||||
* that start at 1900, etc.
|
||||
*/
|
||||
class IgnorableConstantArithmetic extends IgnorableOperation {
|
||||
IgnorableConstantArithmetic() {
|
||||
exists(int i | isLikelyConversionConstant(i) |
|
||||
this.(Operation).getAnOperand().getValue().toInt() = i
|
||||
or
|
||||
// If there is a data flow from the variable that was modified to a function that seems to check for leap year
|
||||
exists(VariableAccess source, ChecksForLeapYearFunctionCall fc |
|
||||
source = var.getAnAccess() and
|
||||
LeapYearCheckFlow::flow(DataFlow::exprNode(source), DataFlow::exprNode(fc.getAnArgument()))
|
||||
)
|
||||
or
|
||||
// If there is a data flow from the field that was modified to a function that seems to check for leap year
|
||||
exists(VariableAccess vacheck, YearFieldAccess yfacheck, ChecksForLeapYearFunctionCall fc |
|
||||
vacheck = var.getAnAccess() and
|
||||
yfacheck.getQualifier() = vacheck and
|
||||
LeapYearCheckFlow::flow(DataFlow::exprNode(yfacheck), DataFlow::exprNode(fc.getAnArgument()))
|
||||
)
|
||||
or
|
||||
// If there is a successor or predecessor that sets the month = 1
|
||||
exists(MonthFieldAccess mfa, AssignExpr ae |
|
||||
mfa.getQualifier() = var.getAnAccess() and
|
||||
mfa.isModified() and
|
||||
(
|
||||
mfa.getBasicBlock() = yfa.getBasicBlock().getASuccessor*() or
|
||||
yfa.getBasicBlock() = mfa.getBasicBlock().getASuccessor+()
|
||||
) and
|
||||
ae = mfa.getEnclosingElement() and
|
||||
ae.getAnOperand().getValue().toInt() = 1
|
||||
exists(AssignArithmeticOperation a | this = a.getRValue() |
|
||||
a.getRValue().getValue().toInt() = i
|
||||
)
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
// If a unary minus assume it is some sort of conversion
|
||||
class IgnorableUnaryMinus extends IgnorableOperation {
|
||||
IgnorableUnaryMinus() {
|
||||
this instanceof UnaryMinusExpr
|
||||
or
|
||||
this.(Operation).getAnOperand() instanceof UnaryMinusExpr
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* An argument to a function is ignorable if the function that is called is an ignored function
|
||||
*/
|
||||
class OperationAsArgToIgnorableFunction extends IgnorableOperation {
|
||||
OperationAsArgToIgnorableFunction() {
|
||||
exists(Call c |
|
||||
c.getAnArgument().getAChild*() = this and
|
||||
c.getTarget() instanceof IgnorableFunction
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Literal OP literal means the result is constant/known
|
||||
* and the operation is basically ignorable (it's not a real operation but
|
||||
* probably one visual simplicity what it means).
|
||||
*/
|
||||
class ConstantBinaryArithmeticOperation extends IgnorableOperation, BinaryArithmeticOperation {
|
||||
ConstantBinaryArithmeticOperation() {
|
||||
this.getLeftOperand() instanceof Literal and
|
||||
this.getRightOperand() instanceof Literal
|
||||
}
|
||||
}
|
||||
|
||||
class IgnorableBinaryBitwiseOperation extends IgnorableOperation instanceof BinaryBitwiseOperation {
|
||||
}
|
||||
|
||||
class IgnorableUnaryBitwiseOperation extends IgnorableOperation instanceof UnaryBitwiseOperation { }
|
||||
|
||||
class IgnorableAssignmentBitwiseOperation extends IgnorableOperation instanceof AssignBitwiseOperation
|
||||
{ }
|
||||
|
||||
/**
|
||||
* Any arithmetic operation where one of the operands is a pointer or char type, ignore it
|
||||
*/
|
||||
class IgnorablePointerOrCharArithmetic extends IgnorableOperation {
|
||||
IgnorablePointerOrCharArithmetic() {
|
||||
this instanceof BinaryArithmeticOperation and
|
||||
(
|
||||
this.(BinaryArithmeticOperation).getAnOperand().getUnspecifiedType() instanceof PointerType
|
||||
or
|
||||
this.(BinaryArithmeticOperation).getAnOperand().getUnspecifiedType() instanceof CharType
|
||||
or
|
||||
// Operations on calls to functions that accept char or char*
|
||||
this.(BinaryArithmeticOperation)
|
||||
.getAnOperand()
|
||||
.(Call)
|
||||
.getAnArgument()
|
||||
.getUnspecifiedType()
|
||||
.stripType() instanceof CharType
|
||||
or
|
||||
// Operations on calls to functions named like "strlen", "wcslen", etc
|
||||
// NOTE: workaround for cases where the wchar_t type is not a char, but an unsigned short
|
||||
// unclear if there is a best way to filter cases like these out based on type info.
|
||||
this.(BinaryArithmeticOperation).getAnOperand().(Call).getTarget().getName().matches("%len%")
|
||||
)
|
||||
or
|
||||
exists(AssignArithmeticOperation a | a.getRValue() = this |
|
||||
a.getAnOperand().getUnspecifiedType() instanceof PointerType
|
||||
or
|
||||
a.getAnOperand().getUnspecifiedType() instanceof CharType
|
||||
or
|
||||
// Operations on calls to functions that accept char or char*
|
||||
a.getAnOperand().(Call).getAnArgument().getUnspecifiedType().stripType() instanceof CharType
|
||||
or
|
||||
// Operations on calls to functions named like "strlen", "wcslen", etc
|
||||
this.(BinaryArithmeticOperation).getAnOperand().(Call).getTarget().getName().matches("%len%")
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* An expression that is a candidate source for an dataflow configuration for an Operation that could flow to a Year field.
|
||||
*/
|
||||
predicate isOperationSourceCandidate(Expr e) {
|
||||
not e instanceof IgnorableOperation and
|
||||
exists(Function f |
|
||||
f = e.getEnclosingFunction() and
|
||||
not f instanceof IgnorableFunction
|
||||
) and
|
||||
(
|
||||
e instanceof SubExpr
|
||||
or
|
||||
e instanceof AddExpr
|
||||
or
|
||||
e instanceof CrementOperation
|
||||
or
|
||||
e instanceof AssignSubExpr
|
||||
or
|
||||
e instanceof AssignAddExpr
|
||||
)
|
||||
select yfa,
|
||||
"Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found.",
|
||||
yfa.getTarget(), yfa.getTarget().toString(), var, var.toString()
|
||||
}
|
||||
|
||||
/**
|
||||
* A dataflow that tracks an ignorable operation (eg. bitwise op) to a operation source, so we may disqualify it.
|
||||
*/
|
||||
module IgnorableOperationToOperationSourceCandidateConfig implements DataFlow::ConfigSig {
|
||||
predicate isSource(DataFlow::Node n) { n.asExpr() instanceof IgnorableOperation }
|
||||
|
||||
predicate isSink(DataFlow::Node n) { isOperationSourceCandidate(n.asExpr()) }
|
||||
|
||||
// looking for sources and sinks in the same function
|
||||
DataFlow::FlowFeature getAFeature() {
|
||||
result instanceof DataFlow::FeatureEqualSourceSinkCallContext
|
||||
}
|
||||
}
|
||||
|
||||
module IgnorableOperationToOperationSourceCandidateFlow =
|
||||
TaintTracking::Global<IgnorableOperationToOperationSourceCandidateConfig>;
|
||||
|
||||
/**
|
||||
* The set of all expressions which is a candidate expression and also does not flow from to to some ignorable expression (eg. bitwise op)
|
||||
* ```
|
||||
* a = something <<< 2;
|
||||
* myDate.year = a + 1; // invalid
|
||||
* ...
|
||||
* a = someDate.year + 1;
|
||||
* myDate.year = a; // valid
|
||||
* ```
|
||||
*/
|
||||
class OperationSource extends Expr {
|
||||
OperationSource() {
|
||||
isOperationSourceCandidate(this) and
|
||||
// If the candidate came from an ignorable operation, ignore the candidate
|
||||
// NOTE: we cannot easily flow the candidate to an ignorable operation as that can
|
||||
// be tricky in practice, e.g., a mod operation on a year would be part of a leap year check
|
||||
// but a mod operation ending in a year is more indicative of something to ignore (a conversion)
|
||||
not exists(IgnorableOperationToOperationSourceCandidateFlow::PathNode sink |
|
||||
sink.getNode().asExpr() = this and
|
||||
sink.isSink()
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
class YearFieldAssignmentNode extends DataFlow::Node {
|
||||
YearFieldAccess access;
|
||||
|
||||
YearFieldAssignmentNode() {
|
||||
exists(Function f |
|
||||
f = this.getEnclosingCallable().getUnderlyingCallable() and not f instanceof IgnorableFunction
|
||||
) and
|
||||
(
|
||||
this.asDefinition().(Assignment).getLValue() = access
|
||||
or
|
||||
this.asDefinition().(CrementOperation).getOperand() = access
|
||||
or
|
||||
exists(Call c | c.getAnArgument() = access and this.asDefiningArgument() = access)
|
||||
or
|
||||
exists(Call c, AddressOfExpr aoe |
|
||||
c.getAnArgument() = aoe and
|
||||
aoe.getOperand() = access and
|
||||
this.asDefiningArgument() = aoe
|
||||
)
|
||||
)
|
||||
}
|
||||
|
||||
YearFieldAccess getYearFieldAccess() { result = access }
|
||||
}
|
||||
|
||||
/**
|
||||
* A DataFlow configuration for identifying flows from some non trivial access or literal
|
||||
* to the Year field of a date object.
|
||||
*/
|
||||
module OperationToYearAssignmentConfig implements DataFlow::ConfigSig {
|
||||
predicate isSource(DataFlow::Node n) { n.asExpr() instanceof OperationSource }
|
||||
|
||||
predicate isSink(DataFlow::Node n) {
|
||||
n instanceof YearFieldAssignmentNode and
|
||||
not isYearModifiedWithCheck(n) and
|
||||
not isControlledByMonthEqualityCheckNonFebruary(n.asExpr())
|
||||
}
|
||||
|
||||
predicate isBarrier(DataFlow::Node n) {
|
||||
exists(ArrayExpr arr | arr.getArrayOffset() = n.asExpr())
|
||||
or
|
||||
n.getType().getUnspecifiedType() instanceof PointerType
|
||||
or
|
||||
n.getType().getUnspecifiedType() instanceof CharType
|
||||
or
|
||||
// If a type resembles "string" ignore flow (likely string conversion, currently ignored)
|
||||
n.getType().getUnspecifiedType().stripType().getName().toLowerCase().matches("%string%")
|
||||
or
|
||||
n.asExpr() instanceof IgnorableOperation
|
||||
or
|
||||
// Flowing into variables that indicate likely non-gregorian years are barriers
|
||||
// e.g., names similar to hijri, persian, lunar, chinese, hebrew, etc.
|
||||
exists(Variable v |
|
||||
v.getName()
|
||||
.toLowerCase()
|
||||
.matches(["%hijri%", "%persian%", "%lunar%", "%chinese%", "%hebrew%"]) and
|
||||
v.getAnAccess() = [n.asIndirectExpr(), n.asExpr()]
|
||||
)
|
||||
or
|
||||
isLeapYearCheckSink(n)
|
||||
or
|
||||
// this is a bit of a hack to address cases where a year is normalized and checked, but the
|
||||
// normalized year is never itself assigned to the final year struct
|
||||
// isLeapYear(getCivilYear(year))
|
||||
// struct.year = year
|
||||
// This is assuming a user would have done this all on one line though.
|
||||
// setting a variable for the conversion and passing that separately would be more difficult to track
|
||||
// considering this approach good enough for current observed false positives
|
||||
exists(Call c, Expr arg |
|
||||
isLeapYearCheckCall(c, arg) and arg.getAChild*() = [n.asExpr(), n.asIndirectExpr()]
|
||||
)
|
||||
or
|
||||
// If as the flow progresses, the value holding a dangerous operation result
|
||||
// is apparently being passed by address to some function, it is more than likely
|
||||
// intended to be modified, and therefore, the definition is killed.
|
||||
exists(Call c | c.getAnArgument().(AddressOfExpr).getAnOperand() = n.asIndirectExpr())
|
||||
}
|
||||
|
||||
/** Block flow out of an operation source to get the "closest" operation to the sink */
|
||||
predicate isBarrierIn(DataFlow::Node n) { isSource(n) }
|
||||
|
||||
predicate isBarrierOut(DataFlow::Node n) { isSink(n) }
|
||||
}
|
||||
|
||||
module OperationToYearAssignmentFlow = TaintTracking::Global<OperationToYearAssignmentConfig>;
|
||||
|
||||
predicate isLeapYearCheckSink(DataFlow::Node sink) {
|
||||
exists(LeapYearGuardCondition lgc |
|
||||
lgc.checkedYearAccess() = [sink.asExpr(), sink.asIndirectExpr()]
|
||||
)
|
||||
or
|
||||
isLeapYearCheckCall(_, [sink.asExpr(), sink.asIndirectExpr()])
|
||||
}
|
||||
|
||||
/**
|
||||
* A flow configuration from a Year field access to some Leap year check or guard
|
||||
*/
|
||||
module YearAssignmentToLeapYearCheckConfig implements DataFlow::ConfigSig {
|
||||
predicate isSource(DataFlow::Node source) { source instanceof YearFieldAssignmentNode }
|
||||
|
||||
predicate isSink(DataFlow::Node sink) { isLeapYearCheckSink(sink) }
|
||||
|
||||
predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
|
||||
// flow from a YearFieldAccess to the qualifier
|
||||
node2.asExpr() = node1.asExpr().(YearFieldAccess).getQualifier*()
|
||||
or
|
||||
// Pass through any intermediate struct
|
||||
exists(Assignment a, DataFlow::PostUpdateNode pun |
|
||||
a.getLValue().(YearFieldAccess).getQualifier*() = pun.getPreUpdateNode().asExpr() and
|
||||
a.getRValue() = node1.asExpr() and
|
||||
node2.asExpr() = a.getLValue().(YearFieldAccess).getQualifier*()
|
||||
)
|
||||
or
|
||||
// flow from a year access qualifier to a year field
|
||||
exists(YearFieldAccess yfa | node2.asExpr() = yfa and node1.asExpr() = yfa.getQualifier())
|
||||
or
|
||||
// in cases of x.year = x and the x is checked, but the year x.year isn't directly
|
||||
// flow from a year assignment node to an RHS if it is an assignment
|
||||
exists(YearFieldAssignmentNode yfan |
|
||||
node1 = yfan and
|
||||
node2.asExpr() = yfan.asDefinition().(Assignment).getRValue()
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
* Enforcing the check must occur in the same call context as the source,
|
||||
* i.e., do not return from the source function and check in a caller.
|
||||
*/
|
||||
DataFlow::FlowFeature getAFeature() { result instanceof DataFlow::FeatureHasSourceCallContext }
|
||||
}
|
||||
|
||||
module YearAssignmentToLeapYearCheckFlow =
|
||||
TaintTracking::Global<YearAssignmentToLeapYearCheckConfig>;
|
||||
|
||||
/** Does there exist a flow from the given YearFieldAccess to a Leap Year check or guard? */
|
||||
predicate isYearModifiedWithCheck(YearFieldAssignmentNode n) {
|
||||
exists(YearAssignmentToLeapYearCheckFlow::PathNode src |
|
||||
src.isSource() and
|
||||
src.getNode() = n
|
||||
)
|
||||
or
|
||||
// If the time flows to a time conversion whose value/result is checked,
|
||||
// assume the leap year is being handled.
|
||||
exists(YearAssignmentToCheckedTimeConversionFlow::PathNode timeQualSrc |
|
||||
timeQualSrc.isSource() and
|
||||
timeQualSrc.getNode() = n
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
* An expression which checks the value of a Month field `a->month == 1`.
|
||||
*/
|
||||
class MonthEqualityCheck extends EqualityOperation {
|
||||
MonthEqualityCheck() { this.getAnOperand() instanceof MonthFieldAccess }
|
||||
|
||||
Expr getExprCompared() {
|
||||
exists(Expr e |
|
||||
e = this.getAnOperand() and
|
||||
not e instanceof MonthFieldAccess and
|
||||
result = e
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
final class FinalMonthEqualityCheck = MonthEqualityCheck;
|
||||
|
||||
class MonthEqualityCheckGuard extends GuardCondition, FinalMonthEqualityCheck { }
|
||||
|
||||
/**
|
||||
* Verifies if the expression is guarded by a check on the Month property of a date struct, that is NOT February.
|
||||
*/
|
||||
bindingset[e]
|
||||
pragma[inline_late]
|
||||
predicate isControlledByMonthEqualityCheckNonFebruary(Expr e) {
|
||||
exists(MonthEqualityCheckGuard monthGuard |
|
||||
monthGuard.controls(e.getBasicBlock(), true) and
|
||||
not monthGuard.getExprCompared().getValueText() = "2"
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
* Flow from a year field access to a time conversion function
|
||||
* that auto converts feb29 in non-leap year, or through a conversion function that doesn't
|
||||
* auto convert to a sanity check guard of the result for error conditions.
|
||||
*/
|
||||
module YearAssignmentToCheckedTimeConversionConfig implements DataFlow::StateConfigSig {
|
||||
class FlowState = boolean;
|
||||
|
||||
predicate isSource(DataFlow::Node source, FlowState state) {
|
||||
source instanceof YearFieldAssignmentNode and
|
||||
state = false
|
||||
}
|
||||
|
||||
predicate isSink(DataFlow::Node sink, FlowState state) {
|
||||
state = true and
|
||||
(
|
||||
exists(IfStmt ifs | ifs.getCondition().getAChild*() = [sink.asExpr(), sink.asIndirectExpr()])
|
||||
or
|
||||
exists(ConditionalExpr ce |
|
||||
ce.getCondition().getAChild*() = [sink.asExpr(), sink.asIndirectExpr()]
|
||||
)
|
||||
or
|
||||
exists(Loop l | l.getCondition().getAChild*() = [sink.asExpr(), sink.asIndirectExpr()])
|
||||
)
|
||||
or
|
||||
state in [true, false] and
|
||||
exists(Call c, TimeConversionFunction f |
|
||||
f.isAutoLeapYearCorrecting() and
|
||||
c.getTarget() = f and
|
||||
c.getAnArgument().getAChild*() = [sink.asExpr(), sink.asIndirectExpr()]
|
||||
)
|
||||
}
|
||||
|
||||
predicate isAdditionalFlowStep(
|
||||
DataFlow::Node node1, FlowState state1, DataFlow::Node node2, FlowState state2
|
||||
) {
|
||||
state1 in [true, false] and
|
||||
state2 = true and
|
||||
exists(Call c |
|
||||
c.getTarget() instanceof TimeConversionFunction and
|
||||
c.getAnArgument().getAChild*() = [node1.asExpr(), node1.asIndirectExpr()] and
|
||||
node2.asExpr() = c
|
||||
)
|
||||
}
|
||||
|
||||
predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
|
||||
// flow from a YearFieldAccess to the qualifier
|
||||
node2.asExpr() = node1.asExpr().(YearFieldAccess).getQualifier*()
|
||||
or
|
||||
node1.(YearFieldAssignmentNode).getYearFieldAccess().getQualifier() = node2.asExpr()
|
||||
or
|
||||
// Pass through any intermediate struct
|
||||
exists(Assignment a, DataFlow::PostUpdateNode pun |
|
||||
a.getLValue().(YearFieldAccess).getQualifier*() = pun.getPreUpdateNode().asExpr() and
|
||||
a.getRValue() = node1.asExpr() and
|
||||
node2.asExpr() = a.getLValue().(YearFieldAccess).getQualifier*()
|
||||
)
|
||||
or
|
||||
// flow from a year access qualifier to a year field
|
||||
exists(YearFieldAccess yfa | node2.asExpr() = yfa and node1.asExpr() = yfa.getQualifier())
|
||||
}
|
||||
|
||||
DataFlow::FlowFeature getAFeature() { result instanceof DataFlow::FeatureHasSourceCallContext }
|
||||
}
|
||||
|
||||
module YearAssignmentToCheckedTimeConversionFlow =
|
||||
DataFlow::GlobalWithState<YearAssignmentToCheckedTimeConversionConfig>;
|
||||
|
||||
/**
|
||||
* Finds flow from a parameter of a function to a leap year check.
|
||||
* This is necessary to handle for scenarios like this:
|
||||
*
|
||||
* year = DANGEROUS_OP // source
|
||||
* isLeap = isLeapYear(year);
|
||||
* // logic based on isLeap
|
||||
* struct.year = year; // sink
|
||||
*
|
||||
* In this case, we may flow a dangerous op to a year assignment, failing
|
||||
* to barrier the flow through a leap year check, as the leap year check
|
||||
* is nested, and dataflow does not progress down into the check and out.
|
||||
* Instead, the point of this flow is to detect isLeapYear's argument
|
||||
* is checked for leap year, making the isLeapYear call a barrier for
|
||||
* the dangerous flow if we flow through the parameter identified to
|
||||
* be checked.
|
||||
*/
|
||||
module ParameterToLeapYearCheckConfig implements DataFlow::ConfigSig {
|
||||
predicate isSource(DataFlow::Node source) { exists(source.asParameter()) }
|
||||
|
||||
predicate isSink(DataFlow::Node sink) {
|
||||
exists(LeapYearGuardCondition lgc |
|
||||
lgc.checkedYearAccess() = [sink.asExpr(), sink.asIndirectExpr()]
|
||||
)
|
||||
}
|
||||
|
||||
predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
|
||||
// flow from a YearFieldAccess to the qualifier
|
||||
node2.asExpr() = node1.asExpr().(YearFieldAccess).getQualifier*()
|
||||
or
|
||||
// flow from a year access qualifier to a year field
|
||||
exists(YearFieldAccess yfa | node2.asExpr() = yfa and node1.asExpr() = yfa.getQualifier())
|
||||
}
|
||||
|
||||
/**
|
||||
* Enforcing the check must occur in the same call context as the source,
|
||||
* i.e., do not return from the source function and check in a caller.
|
||||
*/
|
||||
DataFlow::FlowFeature getAFeature() { result instanceof DataFlow::FeatureHasSourceCallContext }
|
||||
}
|
||||
|
||||
// NOTE: I do not believe taint flow is necessary here as we should
|
||||
// be flowing directyly from some parameter to a leap year check.
|
||||
module ParameterToLeapYearCheckFlow = DataFlow::Global<ParameterToLeapYearCheckConfig>;
|
||||
|
||||
predicate isLeapYearCheckCall(Call c, Expr arg) {
|
||||
exists(ParameterToLeapYearCheckFlow::PathNode src, Function f, int i |
|
||||
src.isSource() and
|
||||
f.getParameter(i) = src.getNode().asParameter() and
|
||||
c.getTarget() = f and
|
||||
c.getArgument(i) = arg
|
||||
)
|
||||
}
|
||||
|
||||
class LeapYearGuardCondition extends GuardCondition {
|
||||
Expr yearSinkDiv4;
|
||||
Expr yearSinkDiv100;
|
||||
Expr yearSinkDiv400;
|
||||
|
||||
LeapYearGuardCondition() {
|
||||
exists(
|
||||
LogicalAndExpr andExpr, LogicalOrExpr orExpr, GuardCondition div4Check,
|
||||
GuardCondition div100Check, GuardCondition div400Check, GuardValue gv
|
||||
|
|
||||
// Cannonical case:
|
||||
// form: `(year % 4 == 0) && (year % 100 != 0 || year % 400 == 0)`
|
||||
// `!((year % 4 == 0) && (year % 100 != 0 || year % 400 == 0))`
|
||||
// `!(year % 4) && (year % 100 || !(year % 400))`
|
||||
// Also accepting `((year & 3) == 0) && (year % 100 != 0 || year % 400 == 0)`
|
||||
// and `(year % 4 == 0) && (year % 100 > 0 || year % 400 == 0)`
|
||||
this = andExpr and
|
||||
andExpr.hasOperands(div4Check, orExpr) and
|
||||
orExpr.hasOperands(div100Check, div400Check) and
|
||||
(
|
||||
// year % 4 == 0
|
||||
exists(RemExpr e |
|
||||
div4Check.comparesEq(e, 0, true, gv) and
|
||||
e.getRightOperand().getValue().toInt() = 4 and
|
||||
yearSinkDiv4 = e.getLeftOperand()
|
||||
)
|
||||
or
|
||||
// year & 3 == 0
|
||||
exists(BitwiseAndExpr e |
|
||||
div4Check.comparesEq(e, 0, true, gv) and
|
||||
e.getRightOperand().getValue().toInt() = 3 and
|
||||
yearSinkDiv4 = e.getLeftOperand()
|
||||
)
|
||||
) and
|
||||
exists(RemExpr e |
|
||||
// year % 100 != 0 or year % 100 > 0
|
||||
(
|
||||
div100Check.comparesEq(e, 0, false, gv) or
|
||||
div100Check.comparesLt(e, 1, false, gv)
|
||||
) and
|
||||
e.getRightOperand().getValue().toInt() = 100 and
|
||||
yearSinkDiv100 = e.getLeftOperand()
|
||||
) and
|
||||
// year % 400 == 0
|
||||
exists(RemExpr e |
|
||||
div400Check.comparesEq(e, 0, true, gv) and
|
||||
e.getRightOperand().getValue().toInt() = 400 and
|
||||
yearSinkDiv400 = e.getLeftOperand()
|
||||
)
|
||||
or
|
||||
// Inverted logic case:
|
||||
// `year % 4 != 0 || (year % 100 == 0 && year % 400 != 0)`
|
||||
// or `year & 3 != 0 || (year % 100 == 0 && year % 400 != 0)`
|
||||
// also accepting `year % 4 > 0 || (year % 100 == 0 && year % 400 > 0)`
|
||||
this = orExpr and
|
||||
orExpr.hasOperands(div4Check, andExpr) and
|
||||
andExpr.hasOperands(div100Check, div400Check) and
|
||||
(
|
||||
// year % 4 != 0 or year % 4 > 0
|
||||
exists(RemExpr e |
|
||||
(
|
||||
div4Check.comparesEq(e, 0, false, gv)
|
||||
or
|
||||
div4Check.comparesLt(e, 1, false, gv)
|
||||
) and
|
||||
e.getRightOperand().getValue().toInt() = 4 and
|
||||
yearSinkDiv4 = e.getLeftOperand()
|
||||
)
|
||||
or
|
||||
// year & 3 != 0
|
||||
exists(BitwiseAndExpr e |
|
||||
div4Check.comparesEq(e, 0, false, gv) and
|
||||
e.getRightOperand().getValue().toInt() = 3 and
|
||||
yearSinkDiv4 = e.getLeftOperand()
|
||||
)
|
||||
) and
|
||||
// year % 100 == 0
|
||||
exists(RemExpr e |
|
||||
div100Check.comparesEq(e, 0, true, gv) and
|
||||
e.getRightOperand().getValue().toInt() = 100 and
|
||||
yearSinkDiv100 = e.getLeftOperand()
|
||||
) and
|
||||
// year % 400 != 0 or year % 400 > 0
|
||||
exists(RemExpr e |
|
||||
(
|
||||
div400Check.comparesEq(e, 0, false, gv)
|
||||
or
|
||||
div400Check.comparesLt(e, 1, false, gv)
|
||||
) and
|
||||
e.getRightOperand().getValue().toInt() = 400 and
|
||||
yearSinkDiv400 = e.getLeftOperand()
|
||||
)
|
||||
)
|
||||
}
|
||||
|
||||
Expr getYearSinkDiv4() { result = yearSinkDiv4 }
|
||||
|
||||
Expr getYearSinkDiv100() { result = yearSinkDiv100 }
|
||||
|
||||
Expr getYearSinkDiv400() { result = yearSinkDiv400 }
|
||||
|
||||
/**
|
||||
* The variable access that is used in all 3 components of the leap year check
|
||||
* e.g., see getYearSinkDiv4/100/400..
|
||||
* If a field access is used, the qualifier and the field access are both returned
|
||||
* in checked condition.
|
||||
* NOTE: if the year is not checked using the same access in all 3 components, no result is returned.
|
||||
* The typical case observed is a consistent variable access is used. If not, this may indicate a bug.
|
||||
* We could check more accurately with a dataflow analysis, but this is likely sufficient for now.
|
||||
*/
|
||||
VariableAccess checkedYearAccess() {
|
||||
exists(Variable var |
|
||||
(
|
||||
this.getYearSinkDiv4().getAChild*() = var.getAnAccess() and
|
||||
this.getYearSinkDiv100().getAChild*() = var.getAnAccess() and
|
||||
this.getYearSinkDiv400().getAChild*() = var.getAnAccess() and
|
||||
result = var.getAnAccess() and
|
||||
(
|
||||
result = this.getYearSinkDiv4().getAChild*() or
|
||||
result = this.getYearSinkDiv100().getAChild*() or
|
||||
result = this.getYearSinkDiv400().getAChild*()
|
||||
)
|
||||
)
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* A difficult case to detect is if a year modification is tied to a month or day modification
|
||||
* and the month or day is safe for leap year.
|
||||
* e.g.,
|
||||
* year++;
|
||||
* month = 1;
|
||||
* // alternative: day = 15;
|
||||
* ... values eventually used in the same time struct
|
||||
* If this is even more challenging if the struct the values end up in are not
|
||||
* local (set inter-procedurally).
|
||||
* This flow flows constants 1-31 to a month or day assignment.
|
||||
* It is assumed a user of this flow will check if the month/day source and month/day sink
|
||||
* are in the same basic blocks as a year modification source and a year modification sink.
|
||||
* It is also assumed a user will check if the constant source is a value that is ignorable
|
||||
* e.g., if it is 2 and the sink is a month assignment, then it isn't ignorable or
|
||||
* if the value is < 27 and is a day assignment, it is likely ignorable
|
||||
*
|
||||
* Obviously this does not handle all conditions (e.g., the month set in another block).
|
||||
* It is meant to capture the most common cases of false positives.
|
||||
*/
|
||||
module CandidateConstantToDayOrMonthAssignmentConfig implements DataFlow::ConfigSig {
|
||||
predicate isSource(DataFlow::Node source) {
|
||||
source.asExpr().getValue().toInt() in [1 .. 31] and
|
||||
(
|
||||
exists(Assignment a | a.getRValue() = source.asExpr())
|
||||
or
|
||||
exists(Call c | c.getAnArgument() = source.asExpr())
|
||||
)
|
||||
}
|
||||
|
||||
predicate isSink(DataFlow::Node sink) {
|
||||
exists(Assignment a |
|
||||
(a.getLValue() instanceof MonthFieldAccess or a.getLValue() instanceof DayFieldAccess) and
|
||||
a.getRValue() = sink.asExpr()
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
// NOTE: only data flow here (no taint tracking) as we want the exact
|
||||
// constant flowing to the month assignment
|
||||
module CandidateConstantToDayOrMonthAssignmentFlow =
|
||||
DataFlow::Global<CandidateConstantToDayOrMonthAssignmentConfig>;
|
||||
|
||||
/**
|
||||
* The value that the assignment resolves to doesn't represent February,
|
||||
* and/or if it represents a day, is a 'safe' day (meaning the 27th or prior).
|
||||
*/
|
||||
bindingset[dayOrMonthValSrcExpr]
|
||||
predicate isSafeValueForAssignmentOfMonthOrDayValue(Assignment a, Expr dayOrMonthValSrcExpr) {
|
||||
a.getLValue() instanceof MonthFieldAccess and
|
||||
dayOrMonthValSrcExpr.getValue().toInt() != 2
|
||||
or
|
||||
a.getLValue() instanceof DayFieldAccess and
|
||||
dayOrMonthValSrcExpr.getValue().toInt() <= 27
|
||||
}
|
||||
|
||||
import OperationToYearAssignmentFlow::PathGraph
|
||||
|
||||
from OperationToYearAssignmentFlow::PathNode src, OperationToYearAssignmentFlow::PathNode sink
|
||||
where
|
||||
OperationToYearAssignmentFlow::flowPath(src, sink) and
|
||||
// Check if a month is set in the same block as the year operation source
|
||||
// and the month value would indicate its set to any other month than february.
|
||||
// Finds if the source year node is in the same block as a source month block
|
||||
// and if the same for the sinks.
|
||||
not exists(DataFlow::Node dayOrMonthValSrc, DataFlow::Node dayOrMonthValSink, Assignment a |
|
||||
CandidateConstantToDayOrMonthAssignmentFlow::flow(dayOrMonthValSrc, dayOrMonthValSink) and
|
||||
a.getRValue() = dayOrMonthValSink.asExpr() and
|
||||
dayOrMonthValSink.getBasicBlock() = sink.getNode().getBasicBlock() and
|
||||
exists(IRBlock dayOrMonthValBB |
|
||||
dayOrMonthValBB = dayOrMonthValSrc.getBasicBlock() and
|
||||
// The source of the day is set in the same block as the source for the year
|
||||
// or the source for the day is set in the same block as the sink for the year
|
||||
dayOrMonthValBB in [
|
||||
src.getNode().getBasicBlock(),
|
||||
sink.getNode().getBasicBlock()
|
||||
]
|
||||
) and
|
||||
isSafeValueForAssignmentOfMonthOrDayValue(a, dayOrMonthValSrc.asExpr())
|
||||
)
|
||||
select sink, src, sink,
|
||||
"Year field has been modified, but no appropriate check for LeapYear was found."
|
||||
|
||||
@@ -44,21 +44,6 @@ class SafeTimeGatheringFunction extends Function {
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* This list of APIs should check for the return value to detect problems during the conversion.
|
||||
*/
|
||||
class TimeConversionFunction extends Function {
|
||||
TimeConversionFunction() {
|
||||
this.getQualifiedName() =
|
||||
[
|
||||
"FileTimeToSystemTime", "SystemTimeToFileTime", "SystemTimeToTzSpecificLocalTime",
|
||||
"SystemTimeToTzSpecificLocalTimeEx", "TzSpecificLocalTimeToSystemTime",
|
||||
"TzSpecificLocalTimeToSystemTimeEx", "RtlLocalTimeToSystemTime",
|
||||
"RtlTimeToSecondsSince1970", "_mkgmtime"
|
||||
]
|
||||
}
|
||||
}
|
||||
|
||||
from FunctionCall fcall, TimeConversionFunction trf, Variable var
|
||||
where
|
||||
fcall = trf.getACallToThisFunction() and
|
||||
|
||||
Reference in New Issue
Block a user