From 73b3e0246a8de7f78d7eb07b1e9d9b7c0b426a61 Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Tue, 2 Dec 2025 11:49:38 +0000 Subject: [PATCH 1/4] C++: Add a small library for control-flow reachability. --- .../UncheckedReturnValueForTimeFunctions.ql | 62 +++++++++++++++++++ 1 file changed, 62 insertions(+) diff --git a/cpp/ql/src/Likely Bugs/Leap Year/UncheckedReturnValueForTimeFunctions.ql b/cpp/ql/src/Likely Bugs/Leap Year/UncheckedReturnValueForTimeFunctions.ql index b223080fb6b3..b05e7b026523 100644 --- a/cpp/ql/src/Likely Bugs/Leap Year/UncheckedReturnValueForTimeFunctions.ql +++ b/cpp/ql/src/Likely Bugs/Leap Year/UncheckedReturnValueForTimeFunctions.ql @@ -14,6 +14,68 @@ import cpp import LeapYear +signature module InputSig { + predicate isSource(ControlFlowNode n); + + predicate isSink(ControlFlowNode n); +} + +module ControlFlowReachability { + pragma[nomagic] + private predicate fwd(ControlFlowNode n) { + Input::isSource(n) + or + exists(ControlFlowNode n0 | + fwd(n0) and + n0.getASuccessor() = n + ) + } + + pragma[nomagic] + private predicate rev(ControlFlowNode n) { + fwd(n) and + ( + Input::isSink(n) + or + exists(ControlFlowNode n1 | + rev(n1) and + n.getASuccessor() = n1 + ) + ) + } + + pragma[nomagic] + private predicate prunedSuccessor(ControlFlowNode n1, ControlFlowNode n2) { + rev(n1) and + rev(n2) and + n1.getASuccessor() = n2 + } + + pragma[nomagic] + predicate isSource(ControlFlowNode n) { + Input::isSource(n) and + rev(n) + } + + pragma[nomagic] + predicate isSink(ControlFlowNode n) { + Input::isSink(n) and + rev(n) + } + + pragma[nomagic] + private predicate successorPlus(ControlFlowNode n1, ControlFlowNode n2) = + doublyBoundedFastTC(prunedSuccessor/2, isSource/1, isSink/1)(n1, n2) + + predicate flowsTo(ControlFlowNode n1, ControlFlowNode n2) { + successorPlus(n1, n2) + or + n1 = n2 and + isSource(n1) and + isSink(n2) + } +} + from FunctionCall fcall, TimeConversionFunction trf, Variable var where fcall = trf.getACallToThisFunction() and From f50bce6307ede223d968ef696b756075e0ff2de6 Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Tue, 2 Dec 2025 11:51:00 +0000 Subject: [PATCH 2/4] C++: Port 'positive' part of the query to the new module. --- .../UncheckedReturnValueForTimeFunctions.ql | 36 +++++++++++++++---- 1 file changed, 30 insertions(+), 6 deletions(-) diff --git a/cpp/ql/src/Likely Bugs/Leap Year/UncheckedReturnValueForTimeFunctions.ql b/cpp/ql/src/Likely Bugs/Leap Year/UncheckedReturnValueForTimeFunctions.ql index b05e7b026523..888a516d1201 100644 --- a/cpp/ql/src/Likely Bugs/Leap Year/UncheckedReturnValueForTimeFunctions.ql +++ b/cpp/ql/src/Likely Bugs/Leap Year/UncheckedReturnValueForTimeFunctions.ql @@ -76,8 +76,7 @@ module ControlFlowReachability { } } -from FunctionCall fcall, TimeConversionFunction trf, Variable var -where +predicate isUnpackedTimeTypeVar(Variable var, FunctionCall fcall, TimeConversionFunction trf) { fcall = trf.getACallToThisFunction() and fcall instanceof ExprInVoidContext and var.getUnderlyingType() instanceof UnpackedTimeType and @@ -91,11 +90,36 @@ where fcall.getAnArgument() = va and var.getAnAccess() = va ) - ) and - exists(DateStructModifiedFieldAccess dsmfa, VariableAccess modifiedVarAccess | + ) +} + +predicate isModifiedFieldAccessToTimeConversionSource( + ControlFlowNode modifiedVarAccess, Variable var +) { + exists(DateStructModifiedFieldAccess dsmfa | + isUnpackedTimeTypeVar(var, _, _) and modifiedVarAccess = var.getAnAccess() and - modifiedVarAccess = dsmfa.getQualifier() and - modifiedVarAccess = fcall.getAPredecessor*() + modifiedVarAccess = dsmfa.getQualifier() + ) +} + +module ModifiedFieldAccessToTimeConversionConfig implements InputSig { + predicate isSource(ControlFlowNode modifiedVarAccess) { + isModifiedFieldAccessToTimeConversionSource(modifiedVarAccess, _) + } + + predicate isSink(ControlFlowNode fcall) { isUnpackedTimeTypeVar(_, fcall, _) } +} + +module ModifiedFieldAccessToTimeConversion = + ControlFlowReachability; + +from FunctionCall fcall, TimeConversionFunction trf, Variable var +where + isUnpackedTimeTypeVar(var, fcall, trf) and + exists(VariableAccess modifiedVarAccess | + isModifiedFieldAccessToTimeConversionSource(modifiedVarAccess, var) and + ModifiedFieldAccessToTimeConversion::flowsTo(modifiedVarAccess, fcall) ) and // Remove false positives not ( From c4723c81bd370c161f727a03885c55d627acf1ee Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Tue, 2 Dec 2025 11:52:22 +0000 Subject: [PATCH 3/4] C++: Port toe fhrst part of the negation to the new module. --- .../UncheckedReturnValueForTimeFunctions.ql | 36 +++++++++++++++---- 1 file changed, 29 insertions(+), 7 deletions(-) diff --git a/cpp/ql/src/Likely Bugs/Leap Year/UncheckedReturnValueForTimeFunctions.ql b/cpp/ql/src/Likely Bugs/Leap Year/UncheckedReturnValueForTimeFunctions.ql index 888a516d1201..7ce42d7bd816 100644 --- a/cpp/ql/src/Likely Bugs/Leap Year/UncheckedReturnValueForTimeFunctions.ql +++ b/cpp/ql/src/Likely Bugs/Leap Year/UncheckedReturnValueForTimeFunctions.ql @@ -114,6 +114,31 @@ module ModifiedFieldAccessToTimeConversionConfig implements InputSig { module ModifiedFieldAccessToTimeConversion = ControlFlowReachability; +module SafeTimeGatheringFunctionCallToTimeConversionFunctionCallConfig implements InputSig { + predicate isSource(ControlFlowNode n) { + n = any(SafeTimeGatheringFunction stgf).getACallToThisFunction() + } + + predicate isSink(ControlFlowNode fcall) { ModifiedFieldAccessToTimeConversion::isSink(fcall) } +} + +module SafeTimeGatheringFunctionCallToTimeConversionFunctionCall = + ControlFlowReachability; + +module SafeTimeGatheringFunctionCallToModifiedFieldAccessConfig implements InputSig { + predicate isSource(ControlFlowNode n) { + n = any(SafeTimeGatheringFunction stgf).getACallToThisFunction() and + SafeTimeGatheringFunctionCallToTimeConversionFunctionCall::isSource(n) + } + + predicate isSink(ControlFlowNode modifiedVarAccess) { + ModifiedFieldAccessToTimeConversion::flowsTo(modifiedVarAccess, _) + } +} + +module SafeTimeGatheringFunctionCallToModifiedFieldAccess = + ControlFlowReachability; + from FunctionCall fcall, TimeConversionFunction trf, Variable var where isUnpackedTimeTypeVar(var, fcall, trf) and @@ -125,13 +150,10 @@ where not ( // Remove any instance where the predecessor is a SafeTimeGatheringFunction and no change to the data happened in between exists(FunctionCall pred | - pred = fcall.getAPredecessor*() and - exists(SafeTimeGatheringFunction stgf | pred = stgf.getACallToThisFunction()) and - not exists(DateStructModifiedFieldAccess dsmfa, VariableAccess modifiedVarAccess | - modifiedVarAccess = var.getAnAccess() and - modifiedVarAccess = dsmfa.getQualifier() and - modifiedVarAccess = fcall.getAPredecessor*() and - modifiedVarAccess = pred.getASuccessor*() + SafeTimeGatheringFunctionCallToTimeConversionFunctionCall::flowsTo(pred, fcall) and + not exists(VariableAccess modifiedVarAccess | + ModifiedFieldAccessToTimeConversion::flowsTo(modifiedVarAccess, fcall) and + SafeTimeGatheringFunctionCallToModifiedFieldAccess::flowsTo(pred, modifiedVarAccess) ) ) or From 682ef759aed97a1035af19d997b84c8b26f5d480 Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Tue, 2 Dec 2025 11:52:52 +0000 Subject: [PATCH 4/4] C++: Port the final part of the negation to the new module. --- .../UncheckedReturnValueForTimeFunctions.ql | 26 ++++++++++++++----- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/cpp/ql/src/Likely Bugs/Leap Year/UncheckedReturnValueForTimeFunctions.ql b/cpp/ql/src/Likely Bugs/Leap Year/UncheckedReturnValueForTimeFunctions.ql index 7ce42d7bd816..dcf6c007fab7 100644 --- a/cpp/ql/src/Likely Bugs/Leap Year/UncheckedReturnValueForTimeFunctions.ql +++ b/cpp/ql/src/Likely Bugs/Leap Year/UncheckedReturnValueForTimeFunctions.ql @@ -139,6 +139,24 @@ module SafeTimeGatheringFunctionCallToModifiedFieldAccessConfig implements Input module SafeTimeGatheringFunctionCallToModifiedFieldAccess = ControlFlowReachability; +module ModifiedMonthFieldAccessToTimeConversionConfig implements InputSig { + predicate isSource(ControlFlowNode n) { + exists(Variable var, MonthFieldAccess mfa, AssignExpr ae | + n = mfa and + isUnpackedTimeTypeVar(var, _, _) and + mfa.getQualifier() = var.getAnAccess() and + mfa.isModified() and + ae = mfa.getEnclosingElement() and + ae.getAnOperand().getValue().toInt() = 1 + ) + } + + predicate isSink(ControlFlowNode fcall) { ModifiedFieldAccessToTimeConversion::flowsTo(_, fcall) } +} + +module ModifiedMonthFieldAccessToTimeConversion = + ControlFlowReachability; + from FunctionCall fcall, TimeConversionFunction trf, Variable var where isUnpackedTimeTypeVar(var, fcall, trf) and @@ -158,13 +176,7 @@ where ) or // Remove any instance where the year is changed, but the month is set to 1 (year wrapping) - exists(MonthFieldAccess mfa, AssignExpr ae | - mfa.getQualifier() = var.getAnAccess() and - mfa.isModified() and - mfa = fcall.getAPredecessor*() and - ae = mfa.getEnclosingElement() and - ae.getAnOperand().getValue().toInt() = 1 - ) + ModifiedMonthFieldAccessToTimeConversion::isSink(fcall) ) select fcall, "$@: Return value of $@ function should be verified to check for any error because variable $@ is not guaranteed to be safe.",