Skip to content

Highly experimental false positive reduction tweaks #314

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

Draft
bdrodes wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Highly experimental false positive reduction tweaks #314

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
83 changes: 65 additions & 18 deletions cpp/ql/src/Likely Bugs/Leap Year/UncheckedLeapYearAfterYearModification.ql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ abstract class YearWriteOp extends Operation {
/**
* A unary operation (Crement) performed on a Year field.
*/
class YearWriteOpUnary extends YearWriteOp, UnaryOperation {
class YearWriteOpUnary extends YearWriteOp, UnaryArithmeticOperation {
YearWriteOpUnary() { this.getOperand() instanceof YearFieldAccess }

override YearFieldAccess getYearAccess() { result = this.getOperand() }
Expand All @@ -86,22 +86,46 @@ class YearWriteOpUnary extends YearWriteOp, UnaryOperation {
/**
* An assignment operation or mutation on the Year field of a date object.
*/
class YearWriteOpAssignment extends YearWriteOp, Assignment {
YearWriteOpAssignment() { this.getLValue() instanceof YearFieldAccess }
class YearWriteOpAssignment extends YearWriteOp, AssignExpr {
Expr mutationExpr;

YearWriteOpAssignment() {
this.getLValue() instanceof YearFieldAccess and
exists(DataFlow::Node source, DataFlow::Node sink |
sink.asExpr() = this.getRValue() and
OperationToYearAssignmentFlow::flow(source, sink) and
mutationExpr = source.asExpr()
) and
not this.getRValue() instanceof LikelyAToIOperation
}

override YearFieldAccess getYearAccess() { result = this.getLValue() }

override Expr getMutationExpr() {
// Note: may need to use DF analysis to pull out the original value,
// if there is excessive false positives.
if this.getOperator() = "="
then
exists(DataFlow::Node source, DataFlow::Node sink |
sink.asExpr() = this.getRValue() and
OperationToYearAssignmentFlow::flow(source, sink) and
result = source.asExpr()
)
else result = this
override Expr getMutationExpr() { result = mutationExpr }
}

class YearArithmetiWriteOpArithmeticAssignment extends YearWriteOp, AssignArithmeticOperation {
YearArithmetiWriteOpArithmeticAssignment() {
this.getLValue() instanceof YearFieldAccess and
not this instanceof AssignRemExpr and
not this.getRValue() instanceof LikelyAToIOperation
}

override YearFieldAccess getYearAccess() { result = this.getLValue() }

override Expr getMutationExpr() { result = this }
}

class LikelyAToIOperation extends Operation {
LikelyAToIOperation() {
this.(SubExpr).getLeftOperand().getUnspecifiedType() instanceof CharType and
this.(SubExpr).getRightOperand().(Literal).getValue().toInt() = 48
or
this.(SubExpr).getRightOperand().(CharLiteral).getValue().toInt() = 48
or
this.(MulExpr).getLeftOperand().(Literal).getValue().toInt() % 10 = 0
or
this instanceof UnaryMinusExpr
}
}

Expand All @@ -111,8 +135,23 @@ class YearWriteOpAssignment extends YearWriteOp, Assignment {
*/
module OperationToYearAssignmentConfig implements DataFlow::ConfigSig {
predicate isSource(DataFlow::Node n) {
not n.asExpr() instanceof Access and
not n.asExpr() instanceof Literal
(
n.asExpr() instanceof BinaryArithmeticOperation
or
n.asExpr() instanceof UnaryArithmeticOperation
) and
not n.asExpr() instanceof LikelyAToIOperation and
not n.asExpr().(MulExpr).getAnOperand() instanceof LikelyAToIOperation and
not n.asExpr() instanceof RemExpr and
not n.asExpr() instanceof UnaryMinusExpr and
// not n.asExpr() instanceof AssignRemExpr and
not n.getType().getUnspecifiedType() instanceof PointerType and
// TODO: or if this is part of an operation
not exists(SubExpr atoi, Literal zero |
n.asExpr() = atoi and
zero = atoi.getAnOperand() and
zero.getValue().toInt() = 0
)
}

predicate isSink(DataFlow::Node n) {
Expand All @@ -121,16 +160,24 @@ module OperationToYearAssignmentConfig implements DataFlow::ConfigSig {
a.getRValue() = n.asExpr()
)
}

predicate isBarrier(DataFlow::Node n) {
exists(ArrayExpr arr | arr.getArrayOffset() = n.asExpr())
or
n.asExpr().getUnspecifiedType() instanceof PointerType
or
n.asExpr() instanceof LikelyAToIOperation
}
}

module OperationToYearAssignmentFlow = DataFlow::Global<OperationToYearAssignmentConfig>;
module OperationToYearAssignmentFlow = TaintTracking::Global<OperationToYearAssignmentConfig>;

from Variable var, YearWriteOp ywo, Expr mutationExpr
where
mutationExpr = ywo.getMutationExpr() and
isYearModifedWithoutExplicitLeapYearCheck(var, ywo) and
not isNormalizationOperation(mutationExpr) and
not ywo instanceof AddressOfExpr and
// not ywo instanceof AddressOfExpr and
not exists(Call c, TimeConversionFunction f | f.getACallToThisFunction() = c |
c.getAnArgument().getAChild*() = var.getAnAccess() and
ywo.getASuccessor*() = c
Expand Down