Merge branch 'master' into taintedallocfp

This commit is contained in:
Jonas Jensen
2019-06-19 15:35:09 +02:00
committed by GitHub
224 changed files with 10236 additions and 785 deletions

View File

@@ -43,11 +43,15 @@ class CastToPointerArithFlow extends DataFlow::Configuration {
}
}
/**
* `derived` has a (possibly indirect) base class of `base`, and at least one new
* field has been introduced in the inheritance chain after `base`.
*/
predicate introducesNewField(Class derived, Class base) {
derived.getABaseClass+() = base and
(
exists(Field f |
f.getDeclaringType() = derived
f.getDeclaringType() = derived and
derived.getABaseClass+() = base
) or
introducesNewField(derived.getABaseClass(), base)
)

View File

@@ -0,0 +1,18 @@
<!DOCTYPE qhelp PUBLIC
"-//Semmle//qhelp//EN"
"qhelp.dtd">
<qhelp>
<overview>
<p>
When eras change, date and time conversions that rely on a hard-coded era start date need to be reviewed. Conversions relying on Japanese dates in the current era can produce an ambiguous date.
The values for the current Japanese era dates should be read from a source that will be updated, such as the Windows registry.
</p>
</overview>
<references>
<li>
<a href="https://blogs.msdn.microsoft.com/shawnste/2018/04/12/the-japanese-calendars-y2k-moment/">The Japanese Calendar's Y2K Moment</a>.
</li>
</references>
</qhelp>

View File

@@ -0,0 +1,19 @@
/**
* @name Hard-coded Japanese era start date
* @description Japanese era changes can lead to code behaving differently. Avoid hard-coding Japanese era start dates.
* @kind problem
* @problem.severity warning
* @id cpp/japanese-era/constructor-or-method-with-exact-era-date
* @precision medium
* @tags reliability
* japanese-era
*/
import cpp
from Call cc, int i
where
cc.getArgument(i).getValue().toInt() = 1989 and
cc.getArgument(i + 1).getValue().toInt() = 1 and
cc.getArgument(i + 2).getValue().toInt() = 8
select cc, "Call that appears to have hard-coded Japanese era start date as parameter."

View File

@@ -0,0 +1,17 @@
<!DOCTYPE qhelp PUBLIC
"-//Semmle//qhelp//EN"
"qhelp.dtd">
<qhelp>
<overview>
<p>
When eras change, date and time conversions that rely on a hard-coded era start date need to be reviewed. Conversions relying on Japanese dates in the current era can produce an ambiguous date.
The values for the current Japanese era dates should be read from a source that will be updated, such as the Windows registry.
</p>
</overview>
<references>
<li>
<a href="https://blogs.msdn.microsoft.com/shawnste/2018/04/12/the-japanese-calendars-y2k-moment/">The Japanese Calendar's Y2K Moment</a>.
</li>
</references>
</qhelp>

View File

@@ -0,0 +1,27 @@
/**
* @name Hard-coded Japanese era start date
* @description Japanese era changes can lead to code behaving differently. Avoid hard-coding Japanese era start dates.
* @kind problem
* @problem.severity warning
* @id cpp/japanese-era/struct-with-exact-era-date
* @tags reliability
* japanese-era
*/
import cpp
import semmle.code.cpp.commons.DateTime
from
StructLikeClass s, YearFieldAccess year, MonthFieldAccess month, DayFieldAccess day,
Operation yearAssignment, Operation monthAssignment, Operation dayAssignment
where
s.getAField().getAnAccess() = year and
yearAssignment.getAnOperand() = year and
yearAssignment.getAnOperand().getValue().toInt() = 1989 and
s.getAField().getAnAccess() = month and
monthAssignment.getAnOperand() = month and
monthAssignment.getAnOperand().getValue().toInt() = 1 and
s.getAField().getAnAccess() = day and
dayAssignment.getAnOperand() = day and
dayAssignment.getAnOperand().getValue().toInt() = 8
select year, "A time struct that is initialized with exact Japanese calendar era start date."

View File

@@ -0,0 +1,22 @@
<!DOCTYPE qhelp PUBLIC
"-//Semmle//qhelp//EN"
"qhelp.dtd">
<qhelp>
<overview>
<include src="LeapYear.qhelp" />
<p>When performing arithmetic operations on a variable that represents a date, leap years must be taken into account.
It is not safe to assume that a year is 365 days long.</p>
</overview>
<recommendation>
<p>Determine whether the time span in question contains a leap day, then perform the calculation using the correct number
of days. Alternatively, use an established library routine that already contains correct leap year logic.</p>
</recommendation>
<references>
<li>U.S. Naval Observatory Website - <a href="https://aa.usno.navy.mil/faq/docs/calendars.php"> Introduction to Calendars</a></li>
<li>Wikipedia - <a href="https://en.wikipedia.org/wiki/Leap_year_bug"> Leap year bug</a> </li>
<li>Microsoft Azure blog - <a href="https://azure.microsoft.com/en-us/blog/is-your-code-ready-for-the-leap-year/"> Is your code ready for the leap year?</a> </li>
</references>
</qhelp>

View File

@@ -0,0 +1,20 @@
/**
* @name Year field changed using an arithmetic operation is used on an unchecked time conversion function
* @description A year field changed using an arithmetic operation is used on a time conversion function, but the return value of the function is not checked for success or failure.
* @kind problem
* @problem.severity warning
* @id cpp/leap-year/adding-365-days-per-year
* @precision medium
* @tags security
* leap-year
*/
import cpp
import LeapYear
import semmle.code.cpp.dataflow.DataFlow
from Expr source, Expr sink, PossibleYearArithmeticOperationCheckConfiguration config
where config.hasFlow(DataFlow::exprNode(source), DataFlow::exprNode(sink))
select sink,
"This arithmetic operation $@ uses a constant value of 365 ends up modifying the date/time located at $@, without considering leap year scenarios.",
source, source.toString(), sink, sink.toString()

View File

@@ -0,0 +1,10 @@
<!DOCTYPE qhelp PUBLIC
"-//Semmle//qhelp//EN"
"qhelp.dtd">
<qhelp>
<fragment>
<p>The leap year rule for the Gregorian calendar, which has become the internationally accepted civil calendar, is: every year that is exactly divisible by four is a leap year, except for years that are exactly divisible by 100, but these centurial years are leap years if they are exactly divisible by 400.</p>
<p>A leap year bug occurs when software (in any language) is written without consideration of leap year logic, or with flawed logic to calculate leap years; which typically results in incorrect results.</p>
<p>The impact of these bugs may range from almost unnoticeable bugs such as an incorrect date, to severe bugs that affect reliability, availability or even the security of the affected system.</p>
</fragment>
</qhelp>

View File

@@ -0,0 +1,276 @@
/**
* Provides a library for helping create leap year related queries
*/
import cpp
import semmle.code.cpp.dataflow.DataFlow
import semmle.code.cpp.controlflow.Guards
import semmle.code.cpp.commons.DateTime
/**
* Get the top-level BinaryOperation enclosing the expression e
*/
BinaryOperation getATopLevelBinaryOperationExpression(Expr e)
{
result = e.getEnclosingElement().(BinaryOperation)
or
result = getATopLevelBinaryOperationExpression( e.getEnclosingElement())
}
/**
* Holds if the top-level binary operation for expression `e` includes the operator specified in `operator` with an operand specified by `valueToCheck`
*/
predicate additionalLogicalCheck( Expr e, string operation, int valueToCheck) {
exists(BinaryLogicalOperation bo |
bo = getATopLevelBinaryOperationExpression(e) |
exists( BinaryArithmeticOperation bao |
bao = bo.getAChild*() |
bao.getAnOperand().getValue().toInt() = valueToCheck
and bao.getOperator() = operation
)
)
}
/**
* Operation that seems to be checking for leap year
*/
class CheckForLeapYearOperation extends Operation {
CheckForLeapYearOperation() {
exists( BinaryArithmeticOperation bo |
bo = this |
bo.getAnOperand().getValue().toInt() = 4
and bo.getOperator().toString() = "%"
and additionalLogicalCheck( this.getEnclosingElement(), "%", 100)
and additionalLogicalCheck( this.getEnclosingElement(), "%", 400)
)
}
override string getOperator() { result = "LeapYearCheck" }
}
/**
* abstract class of type YearFieldAccess that would represent an access to a year field on a struct and is used for arguing about leap year calculations
*/
abstract class LeapYearFieldAccess extends YearFieldAccess {
/**
* Holds if the field access is a modification,
* and it involves an arithmetic operation
*/
predicate isModifiedByArithmeticOperation() {
this.isModified()
and exists( Operation op |
op.getAnOperand() = this
and ( op instanceof AssignArithmeticOperation
or exists( BinaryArithmeticOperation bao |
bao = op.getAnOperand())
or op instanceof PostfixCrementOperation
or op instanceof PrefixCrementOperation
)
)
}
/**
* Holds if the field access is a modification,
* and it involves an arithmetic operation.
* In order to avoid false positives, the operation does not includes values that are normal for year normalization.
*
* 1900 - struct tm counts years since 1900
* 1980/80 - FAT32 epoch
*/
predicate isModifiedByArithmeticOperationNotForNormalization() {
this.isModified()
and exists( Operation op |
op.getAnOperand() = this
and ( (op instanceof AssignArithmeticOperation
and not ( op.getAChild().getValue().toInt() = 1900
or op.getAChild().getValue().toInt() = 2000
or op.getAChild().getValue().toInt() = 1980
or op.getAChild().getValue().toInt() = 80
// Special case for transforming marshaled 2-digit year date:
// theTime.wYear += 100*value;
or exists( MulExpr mulBy100 | mulBy100 = op.getAChild() |
mulBy100.getAChild().getValue().toInt() = 100 )))
or exists( BinaryArithmeticOperation bao |
bao = op.getAnOperand()
and not ( bao.getAChild().getValue().toInt() = 1900
or bao.getAChild().getValue().toInt() = 2000
or bao.getAChild().getValue().toInt() = 1980
or bao.getAChild().getValue().toInt() = 80
// Special case for transforming marshaled 2-digit year date:
// theTime.wYear += 100*value;
or exists( MulExpr mulBy100 | mulBy100 = op.getAChild() |
mulBy100.getAChild().getValue().toInt() = 100 ))
)
or op instanceof PostfixCrementOperation
or op instanceof PrefixCrementOperation
)
)
}
/**
* Holds if the top-level binary operation includes a modulus operator with an operand specified by `valueToCheck`
*/
predicate additionalModulusCheckForLeapYear( int valueToCheck) {
additionalLogicalCheck(this, "%", valueToCheck)
}
/**
* Holds if the top-level binary operation includes an addition or subtraction operator with an operand specified by `valueToCheck`
*/
predicate additionalAdditionOrSubstractionCheckForLeapYear( int valueToCheck) {
additionalLogicalCheck(this, "+", valueToCheck)
or additionalLogicalCheck(this, "-", valueToCheck)
}
/**
* Holds true if this object is used on a modulus 4 operation, which would likely indicate the start of a leap year check
*/
predicate isUsedInMod4Operation()
{
not this.isModified() and
exists( BinaryArithmeticOperation bo |
bo.getAnOperand() = this
and bo.getAnOperand().getValue().toInt() = 4
and bo.getOperator().toString() = "%"
)
}
/**
* Holds true if this object seems to be used in a valid gregorian calendar leap year check
*/
predicate isUsedInCorrectLeapYearCheck()
{
// The Gregorian leap year rule is:
// Every year that is exactly divisible by four is a leap year,
// except for years that are exactly divisible by 100,
// but these centurial years are leap years if they are exactly divisible by 400
//
// https://aa.usno.navy.mil/faq/docs/calendars.php
this.isUsedInMod4Operation()
and additionalModulusCheckForLeapYear(400)
and additionalModulusCheckForLeapYear(100)
}
}
/**
* YearFieldAccess for SYSTEMTIME struct
*/
class StructSystemTimeLeapYearFieldAccess extends LeapYearFieldAccess {
StructSystemTimeLeapYearFieldAccess() {
this.toString().matches("wYear")
}
}
/**
* YearFieldAccess for struct tm
*/
class StructTmLeapYearFieldAccess extends LeapYearFieldAccess {
StructTmLeapYearFieldAccess() {
this.toString().matches("tm_year")
}
override predicate isUsedInCorrectLeapYearCheck()
{
this.isUsedInMod4Operation()
and additionalModulusCheckForLeapYear(400)
and additionalModulusCheckForLeapYear(100)
// tm_year represents years since 1900
and ( additionalAdditionOrSubstractionCheckForLeapYear(1900)
// some systems may use 2000 for 2-digit year conversions
or additionalAdditionOrSubstractionCheckForLeapYear(2000)
// converting from/to Unix epoch
or additionalAdditionOrSubstractionCheckForLeapYear(1970)
)
}
}
/**
* FunctionCall that includes an operation that is checking for leap year
*/
class ChecksForLeapYearFunctionCall extends FunctionCall {
ChecksForLeapYearFunctionCall() {
exists( Function f, CheckForLeapYearOperation clyo |
f.getACallToThisFunction() = this |
clyo.getEnclosingFunction() = f
)
}
}
/**
* DataFlow::Configuration for finding a variable access that would flow into
* a function call that includes an operation to check for leap year
*/
class LeapYearCheckConfiguration extends DataFlow::Configuration {
LeapYearCheckConfiguration() {
this = "LeapYearCheckConfiguration"
}
override predicate isSource(DataFlow::Node source) {
exists( VariableAccess va |
va = source.asExpr()
)
}
override predicate isSink(DataFlow::Node sink) {
exists( ChecksForLeapYearFunctionCall fc |
sink.asExpr() = fc.getAnArgument()
)
}
}
/**
* DataFlow::Configuration for finding an operation w/hardcoded 365 that will flow into a FILEINFO field
*/
class FiletimeYearArithmeticOperationCheckConfiguration extends DataFlow::Configuration {
FiletimeYearArithmeticOperationCheckConfiguration() {
this = "FiletimeYearArithmeticOperationCheckConfiguration"
}
override predicate isSource(DataFlow::Node source) {
exists( Expr e, Operation op |
e = source.asExpr() |
op.getAChild*().getValue().toInt()=365
and op.getAChild*() = e
)
}
override predicate isSink(DataFlow::Node sink) {
exists( StructLikeClass dds, FieldAccess fa, AssignExpr aexpr, Expr e |
e = sink.asExpr() |
dds instanceof FileTimeStruct
and fa.getQualifier().getUnderlyingType() = dds
and fa.isModified()
and aexpr.getAChild() = fa
and aexpr.getChild(1).getAChild*() = e
)
}
}
/**
* DataFlow::Configuration for finding an operation w/hardcoded 365 that will flow into any known date/time field
*/
class PossibleYearArithmeticOperationCheckConfiguration extends DataFlow::Configuration {
PossibleYearArithmeticOperationCheckConfiguration() {
this = "PossibleYearArithmeticOperationCheckConfiguration"
}
override predicate isSource(DataFlow::Node source) {
exists( Expr e, Operation op |
e = source.asExpr() |
op.getAChild*().getValue().toInt()=365
and op.getAChild*() = e
)
}
override predicate isSink(DataFlow::Node sink) {
exists( StructLikeClass dds, FieldAccess fa, AssignExpr aexpr, Expr e |
e = sink.asExpr() |
(dds instanceof FileTimeStruct
or dds instanceof DateDataStruct)
and fa.getQualifier().getUnderlyingType() = dds
and fa.isModified()
and aexpr.getAChild() = fa
and aexpr.getChild(1).getAChild*() = e
)
}
}

View File

@@ -0,0 +1,29 @@
<!DOCTYPE qhelp PUBLIC
"-//Semmle//qhelp//EN"
"qhelp.dtd">
<qhelp>
<overview>
<include src="LeapYear.qhelp" />
<p>When performing arithmetic operations on a variable that represents a year, it is important to consider that the resulting value may not be a valid date.</p>
<p>The typical example is doing simple year arithmetic (i.e. <code>date.year++</code>) without considering if the resulting value will be a valid date or not.</p>
</overview>
<recommendation>
<p>When modifying a year field on a date structure, verify if the resulting year is a leap year.</p>
</recommendation>
<example>
<p>In this example, we are adding 1 year to the current date. This may work most of the time, but on any given February 29th, the resulting value will be invalid.</p>
<sample src="UncheckedLeapYearAfterYearModificationBad.c" />
<p>To fix this bug, check the result for leap year.</p>
<sample src="UncheckedLeapYearAfterYearModificationGood.c" />
</example>
<references>
<li>U.S. Naval Observatory Website - <a href="https://aa.usno.navy.mil/faq/docs/calendars.php"> Introduction to Calendars</a></li>
<li>Wikipedia - <a href="https://en.wikipedia.org/wiki/Leap_year_bug"> Leap year bug</a> </li>
<li>Microsoft Azure blog - <a href="https://azure.microsoft.com/en-us/blog/is-your-code-ready-for-the-leap-year/"> Is your code ready for the leap year?</a> </li>
</references>
</qhelp>

View File

@@ -0,0 +1,64 @@
/**
* @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
* @problem.severity warning
* @id cpp/leap-year/unchecked-after-arithmetic-year-modification
* @precision medium
* @tags security
* leap-year
*/
import cpp
import LeapYear
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 = yfa.getASuccessor*()
)
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, LeapYearCheckConfiguration config
|
source = var.getAnAccess() and
config.hasFlow(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,
LeapYearCheckConfiguration config
|
vacheck = var.getAnAccess() and
yfacheck.getQualifier() = vacheck and
config.hasFlow(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 = yfa.getASuccessor*() or
yfa = mfa.getASuccessor*()
) and
ae = mfa.getEnclosingElement() and
ae.getAnOperand().getValue().toInt() = 1
)
)
)
select yfa,
"Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found.",
yfa.getTarget(), yfa.getTarget().toString(), var, var.toString()

View File

@@ -0,0 +1,9 @@
SYSTEMTIME st;
FILETIME ft;
GetSystemTime(&st);
// Flawed logic may result in invalid date
st.wYear++;
// The following code may fail
SystemTimeToFileTime(&st, &ft);

View File

@@ -0,0 +1,15 @@
SYSTEMTIME st;
FILETIME ft;
GetSystemTime(&st);
// Flawed logic will result in invalid date
st.wYear++;
// Check for leap year, and adjust the date accordingly
bool isLeapYear = st.wYear % 4 == 0 && (st.wYear % 100 != 0 || st.wYear % 400 == 0);
st.wDay = st.wMonth == 2 && st.wDay == 29 && !isLeapYear ? 28 : st.wDay;
if (!SystemTimeToFileTime(&st, &ft))
{
// handle error
}

View File

@@ -0,0 +1,41 @@
<!DOCTYPE qhelp PUBLIC
"-//Semmle//qhelp//EN"
"qhelp.dtd">
<qhelp>
<overview>
<include src="LeapYear.qhelp" />
<p>When using a function that transforms a date structure, and the year on the input argument for the API has been manipulated, it is important to check for the return value of the function to make sure it succeeded.</p>
<p>Otherwise, the function may have failed, and the output parameter may contain invalid data that can cause any number of problems on the affected system.</p>
<p>The following is a list of the functions that this query covers:</p>
<ul>
<li><code>FileTimeToSystemTime</code></li>
<li><code>SystemTimeToFileTime</code></li>
<li><code>SystemTimeToTzSpecificLocalTime</code></li>
<li><code>SystemTimeToTzSpecificLocalTimeEx</code></li>
<li><code>TzSpecificLocalTimeToSystemTime</code></li>
<li><code>TzSpecificLocalTimeToSystemTimeEx</code></li>
<li><code>RtlLocalTimeToSystemTime</code></li>
<li><code>RtlTimeToSecondsSince1970</code></li>
<li><code>_mkgmtime</code></li>
</ul>
</overview>
<recommendation>
<p>When calling an API that transforms a date variable that was manipulated, always check for the return value to verify that the API call succeeded.</p>
</recommendation>
<example>
<p>In this example, we are adding 1 year to the current date. This may work most of the time, but on any given February 29th, the resulting value will be invalid.</p>
<sample src="UncheckedLeapYearAfterYearModificationBad.c" />
<p>To fix this bug, you must verify the return value for <code>SystemTimeToFileTime</code> and handle any potential error accordingly.</p>
<sample src="UncheckedLeapYearAfterYearModificationGood.c" />
</example>
<references>
<li>U.S. Naval Observatory Website - <a href="https://aa.usno.navy.mil/faq/docs/calendars.php"> Introduction to Calendars</a></li>
<li>Wikipedia - <a href="https://en.wikipedia.org/wiki/Leap_year_bug"> Leap year bug</a> </li>
<li>Microsoft Azure blog - <a href="https://azure.microsoft.com/en-us/blog/is-your-code-ready-for-the-leap-year/"> Is your code ready for the leap year?</a> </li>
</references>
</qhelp>

View File

@@ -0,0 +1,110 @@
/**
* @name Year field changed using an arithmetic operation is used on an unchecked time conversion function
* @description A year field changed using an arithmetic operation is used on a time conversion function, but the return value of the function is not checked for success or failure
* @kind problem
* @problem.severity warning
* @id cpp/leap-year/unchecked-return-value-for-time-conversion-function
* @precision medium
* @tags security
* leap-year
*/
import cpp
import LeapYear
/**
* A YearFieldAccess that is modifying the year by any arithmetic operation
*
* NOTE:
* To change this class to work for general purpose date transformations that do not check the return value,
* make the following changes:
* -> extends FieldAccess (line 27)
* -> this.isModified (line 33)
* Expect a lower precision for a general purpose version.
*/
class DateStructModifiedFieldAccess extends LeapYearFieldAccess {
DateStructModifiedFieldAccess() {
exists(Field f, StructLikeClass struct |
f.getAnAccess() = this and
struct.getAField() = f and
struct.getUnderlyingType() instanceof DateDataStruct and
this.isModifiedByArithmeticOperation()
)
}
}
/**
* This is a list of APIs that will get the system time, and therefore guarantee that the value is valid
*/
class SafeTimeGatheringFunction extends Function {
SafeTimeGatheringFunction() {
this.getQualifiedName().matches("GetFileTime") or
this.getQualifiedName().matches("GetSystemTime") or
this.getQualifiedName().matches("NtQuerySystemTime")
}
}
/**
* This list of APIs should check for the return value to detect problems during the conversion
*/
class TimeConversionFunction extends Function {
TimeConversionFunction() {
this.getQualifiedName().matches("FileTimeToSystemTime") or
this.getQualifiedName().matches("SystemTimeToFileTime") or
this.getQualifiedName().matches("SystemTimeToTzSpecificLocalTime") or
this.getQualifiedName().matches("SystemTimeToTzSpecificLocalTimeEx") or
this.getQualifiedName().matches("TzSpecificLocalTimeToSystemTime") or
this.getQualifiedName().matches("TzSpecificLocalTimeToSystemTimeEx") or
this.getQualifiedName().matches("RtlLocalTimeToSystemTime") or
this.getQualifiedName().matches("RtlTimeToSecondsSince1970") or
this.getQualifiedName().matches("_mkgmtime")
}
}
from FunctionCall fcall, TimeConversionFunction trf, Variable var
where
fcall = trf.getACallToThisFunction() and
fcall instanceof ExprInVoidContext and
var.getUnderlyingType() instanceof DateDataStruct and
(
exists(AddressOfExpr aoe |
aoe = fcall.getAnArgument() and
aoe.getAddressable() = var
)
or
exists(VariableAccess va |
fcall.getAnArgument() = va and
var.getAnAccess() = va
)
) and
exists(DateStructModifiedFieldAccess dsmfa, VariableAccess modifiedVarAccess |
modifiedVarAccess = var.getAnAccess() and
modifiedVarAccess = dsmfa.getQualifier() and
modifiedVarAccess = fcall.getAPredecessor*()
) and
// Remove false positives
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*()
)
)
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
)
)
select fcall,
"Return value of $@ function should be verified to check for any error because variable $@ is not guaranteed to be safe.",
trf, trf.getQualifiedName().toString(), var, var.getName()

View File

@@ -0,0 +1,30 @@
<!DOCTYPE qhelp PUBLIC
"-//Semmle//qhelp//EN"
"qhelp.dtd">
<qhelp>
<overview>
<include src="LeapYear.qhelp" />
<p>This query helps to detect when a developer allocates an array or other fixed-length data structure such as <code>std::vector</code> with 365 elements one for each day of the year.</p>
<p>Since leap years have 366 days, there will be no allocated element on December 31st at the end of a leap year; which will lead to a buffer overflow on a leap year.</p>
</overview>
<recommendation>
<p>When allocating memory for storing elements for each day of the year, ensure that the correct number of elements are allocated.</p>
<p>It is also highly recommended to compile the code with array-bounds checking enabled whenever possible.</p>
</recommendation>
<example>
<p>In this example, we are allocating 365 integers, one for each day of the year. This code will fail on a leap year, when there are 366 days.</p>
<sample src="UnsafeArrayForDaysOfYearBad.c" />
<p>When using arrays, allocate the correct number of elements to match the year.</p>
<sample src="UnsafeArrayForDaysOfYearGood.c" />
</example>
<references>
<li>U.S. Naval Observatory Website - <a href="https://aa.usno.navy.mil/faq/docs/calendars.php"> Introduction to Calendars</a></li>
<li>Wikipedia - <a href="https://en.wikipedia.org/wiki/Leap_year_bug"> Leap year bug</a> </li>
<li>Microsoft Azure blog - <a href="https://azure.microsoft.com/en-us/blog/is-your-code-ready-for-the-leap-year/"> Is your code ready for the leap year?</a> </li>
</references>
</qhelp>

View File

@@ -0,0 +1,37 @@
/**
* @name Unsafe array for days of the year
* @description An array of 365 items typically indicates one entry per day of the year, but without considering leap years, which would be 366 days.
* An access on a leap year could result in buffer overflow bugs.
* @kind problem
* @problem.severity warning
* @id cpp/leap-year/unsafe-array-for-days-of-the-year
* @precision medium
* @tags security
* leap-year
*/
import cpp
class LeapYearUnsafeDaysOfTheYearArrayType extends ArrayType {
LeapYearUnsafeDaysOfTheYearArrayType() { this.getArraySize() = 365 }
}
from Element element
where
exists(NewArrayExpr nae |
element = nae and
nae.getAllocatedType() instanceof LeapYearUnsafeDaysOfTheYearArrayType
)
or
exists(Variable var |
var = element and
var.getType() instanceof LeapYearUnsafeDaysOfTheYearArrayType
)
or
exists(ConstructorCall cc |
element = cc and
cc.getTarget().hasName("vector") and
cc.getArgument(0).getValue().toInt() = 365
)
select element,
"There is an array or std::vector allocation with a hard-coded set of 365 elements, which may indicate the number of days in a year without considering leap year scenarios."

View File

@@ -0,0 +1,2 @@
int items[365];
items[dayOfYear - 1] = x; // buffer overflow on December 31st of any leap year

View File

@@ -0,0 +1,4 @@
bool isLeapYear = year % 4 == 0 && (year % 100 != 0 || year % 400 == 0);
int *items = new int[isLeapYear ? 366 : 365];
// ...
delete[] items;

View File

@@ -55,6 +55,7 @@ predicate memberDirectlyInitialisesVariable(MemberFunction mf, Class c, MemberVa
predicate memberInitialisesVariable(MemberFunction mf, Class c, MemberVariable mv) {
memberDirectlyInitialisesVariable(mf, c, mv) or
exists(MemberFunction mf2 |
memberDirectlyInitialisesVariable(_, c, mv) and // (optimizer hint)
memberInitialisesVariable(mf2, c, mv) and
mf.getDeclaringType() = c and
mf.calls(mf2)

View File

@@ -9,20 +9,27 @@
* readability
* external/jsf
*/
import cpp
from Parameter p, Parameter superP, MemberFunction subF, MemberFunction superF, int i, string subValue, string superValue
where p.hasInitializer()
and subF.getParameter(i) = p
and superP.hasInitializer()
and subF.overrides(superF)
and superF.getParameter(i) = superP
and subValue = p.getInitializer().getExpr().getValue()
and superValue = superP.getInitializer().getExpr().getValue()
and subValue != superValue
select p.getInitializer().getExpr(),
"Parameter " + p.getName() +
" redefines its default value to " + subValue +
" from the inherited default value " + superValue +
" (in " + superF.getDeclaringType().getName() +
").\nThe default value will be resolved statically, not by dispatch, so this can cause confusion."
predicate memberParameterWithDefault(
MemberFunction f, int ix, Parameter p, Expr initExpr, string initValue
) {
f.getParameter(ix) = p and
initExpr = p.getInitializer().getExpr() and
initValue = initExpr.getValue()
}
from
Parameter p, Parameter superP, MemberFunction subF, MemberFunction superF, int i, Expr subExpr,
string subValue, string superValue
where
memberParameterWithDefault(subF, i, p, subExpr, subValue) and
subF.overrides(superF) and
memberParameterWithDefault(superF, i, superP, _, superValue) and
subValue != superValue
select subExpr,
"Parameter " + p.getName() + " redefines its default value to " + subValue +
" from the inherited default value " + superValue + " (in " +
superF.getDeclaringType().getName() +
").\nThe default value will be resolved statically, not by dispatch, so this can cause confusion."

View File

@@ -54,7 +54,7 @@ class Field extends MemberVariable {
* which the field will be initialized, whether by an initializer list or in a
* constructor.
*/
final int getInitializationOrder() {
final pragma[nomagic] int getInitializationOrder() {
exists(Class cls, int memberIndex |
this = cls.getCanonicalMember(memberIndex) and
memberIndex = rank[result + 1](int index |

View File

@@ -117,6 +117,11 @@ class Variable extends Declaration, @variable {
or
exists (AssignExpr ae
| ae.getLValue().(Access).getTarget() = this and result = ae.getRValue())
or
exists(AggregateLiteral l |
this.getDeclaringType() = l.getType() and
result = l.getChild(this.(Field).getInitializationOrder())
)
}
/**

View File

@@ -0,0 +1,110 @@
/**
* Provides a library for helping working with a set of known data structures representing dates in C++
*/
import cpp
class FileTimeStruct extends Type {
FileTimeStruct() {
this.toString().matches("_FILETIME")
or this.toString().matches("_FILETIME %")
}
}
/**
* Type of known data structures that are used for date representation.
*/
class DateDataStruct extends Type {
DateDataStruct() {
this.toString().matches("_SYSTEMTIME")
or this.toString().matches("SYSTEMTIME")
or this.toString().matches("tm")
or this.toString().matches("_SYSTEMTIME %")
or this.toString().matches("SYSTEMTIME %")
or this.toString().matches("tm %")
}
}
/**
* abstract class of type FieldAccess that would represent an access to a field on a struct
*/
abstract class StructFieldAccess extends FieldAccess {
StructFieldAccess () {
exists(Field f, StructLikeClass struct |
f.getAnAccess() = this
and struct.getAField() = f
)
}
}
/**
* abstract class of type FieldAccess where access is to a day of the month field of the struct
* This is to be derived from for a specific struct's day of the month field access
*/
abstract class DayFieldAccess extends StructFieldAccess { }
/**
* abstract class of type FieldAccess where access is to a month field of the struct
* This is to be derived from for a specific struct's month field access
*/
abstract class MonthFieldAccess extends StructFieldAccess {}
/**
* abstract class of type FieldAccess where access is to a year field of the struct
* This is to be derived from for a specific struct's year field access
*/
abstract class YearFieldAccess extends StructFieldAccess {}
/**
* DayFieldAccess for SYSTEMTIME struct
*/
class SystemTimeDayFieldAccess extends DayFieldAccess {
SystemTimeDayFieldAccess () {
this.toString().matches("wDay")
}
}
/**
* MonthFieldAccess for SYSTEMTIME struct
*/
class SystemTimeMonthFieldAccess extends MonthFieldAccess {
SystemTimeMonthFieldAccess () {
this.toString().matches("wMonth")
}
}
/**
* YearFieldAccess for SYSTEMTIME struct
*/
class StructSystemTimeYearFieldAccess extends YearFieldAccess {
StructSystemTimeYearFieldAccess() {
this.toString().matches("wYear")
}
}
/**
* DayFieldAccess for struct tm
*/
class StructTmDayFieldAccess extends DayFieldAccess {
StructTmDayFieldAccess() {
this.toString().matches("tm_mday")
}
}
/**
* MonthFieldAccess for struct tm
*/
class StructTmMonthFieldAccess extends MonthFieldAccess {
StructTmMonthFieldAccess() {
this.toString().matches("tm_mon")
}
}
/**
* YearFieldAccess for struct tm
*/
class StructTmYearFieldAccess extends YearFieldAccess {
StructTmYearFieldAccess() {
this.toString().matches("tm_year")
}
}

View File

@@ -131,20 +131,28 @@ private predicate bbLoopEntryConditionAlwaysTrueAt(BasicBlock bb, int i, Control
}
/**
* Basic block `pred` ends with a condition belonging to a loop, and that
* condition is provably true upon entry. Basic block `succ` is a successor
* of `pred`, and `skipsLoop` indicates whether `succ` is the false-successor
* of `pred`.
* Basic block `pred` contains all or part of the condition belonging to a loop,
* and there is an edge from `pred` to `succ` that concludes the condition.
* If the edge corrseponds with the loop condition being found to be `true`, then
* `skipsLoop` is `false`. Otherwise the edge corresponds with the loop condition
* being found to be `false` and `skipsLoop` is `true`. Non-concluding edges
* within a complex loop condition are not matched by this predicate.
*/
private predicate bbLoopConditionAlwaysTrueUponEntrySuccessor(BasicBlock pred, BasicBlock succ, boolean skipsLoop) {
succ = pred.getASuccessor() and
exists(ControlFlowNode last |
last = pred.getEnd() and
loopConditionAlwaysTrueUponEntry(_, last) and
if succ = pred.getAFalseSuccessor() then
skipsLoop = true
else
skipsLoop = false
exists(Expr cond |
loopConditionAlwaysTrueUponEntry(_, cond) and
cond.getAChild*() = pred.getEnd() and
succ = pred.getASuccessor() and
not cond.getAChild*() = succ.getStart() and
(
(
succ = pred.getAFalseSuccessor() and
skipsLoop = true
) or (
succ = pred.getATrueSuccessor() and
skipsLoop = false
)
)
)
}
@@ -176,7 +184,7 @@ predicate bbSuccessorEntryReachesLoopInvariant(BasicBlock pred, BasicBlock succ,
// The edge from `pred` to `succ` is _not_ from a loop condition provably
// true upon entry, so the values of `predSkipsFirstLoopAlwaysTrueUponEntry`
// and `succSkipsFirstLoopAlwaysTrueUponEntry` must be the same.
not bbLoopConditionAlwaysTrueUponEntrySuccessor(pred, _, _) and
not bbLoopConditionAlwaysTrueUponEntrySuccessor(pred, succ, _) and
succSkipsFirstLoopAlwaysTrueUponEntry = predSkipsFirstLoopAlwaysTrueUponEntry and
// Moreover, if `pred` contains the entry point of a loop where the
// condition is provably true upon entry, then `succ` is not allowed

View File

@@ -60,27 +60,14 @@ private cached module Cached {
not n2 instanceof PrimitiveBasicBlock
}
/** Returns the index of `node` in its `PrimitiveBasicBlock`. */
private int getMemberIndex(Node node) {
primitive_basic_block_entry_node(node) and
result = 0
or
exists(Node prev |
member_step(prev, node) and
result = getMemberIndex(prev) + 1
)
}
/** Holds if `node` is the `pos`th control-flow node in primitive basic block `bb`. */
cached
predicate primitive_basic_block_member(Node node, PrimitiveBasicBlock bb, int pos) {
primitive_basic_block_entry_node(bb) and
(
pos = 0 and
node = bb
or
pos = getMemberIndex(node) and
member_step+(bb, node)
primitive_basic_block_entry_node(bb) and node = bb and pos = 0
or
exists(Node prev |
member_step(prev, node) and
primitive_basic_block_member(prev, bb, pos - 1)
)
}

View File

@@ -45,6 +45,10 @@ class Stmt extends StmtParent, @stmt {
/**
* Gets the statement following this statement in the same block, if any.
*
* Note that this is not widely useful, because this doesn't have a result for
* the last statement of a block. Consider using the `ControlFlowNode` class
* to trace the flow of control instead.
*/
Stmt getFollowingStmt() {
exists(Block b, int i | this = b.getStmt(i) and