Merge pull request #1354 from denislevin/denisl/cpp/MishandlingJapaneseDatesAndLeapYear

C++: Mishandling Japanese Era and Leap Year in calculations

cpp/ql/src/Likely Bugs/JapaneseEra/ConstructorOrMethodWithExactEraDate.qhelp

@@ -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<61>s Y2K Moment</a>.
+  </li>
+</references>
+</qhelp>
+

cpp/ql/src/Likely Bugs/JapaneseEra/ConstructorOrMethodWithExactEraDate.ql

@@ -0,0 +1,17 @@
+/**
+ * @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." 

cpp/ql/src/Likely Bugs/JapaneseEra/StructWithExactEraDate.qhelp

@@ -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<61>s Y2K Moment</a>.
+  </li>
+</references>
+</qhelp>

cpp/ql/src/Likely Bugs/JapaneseEra/StructWithExactEraDate.ql

@@ -0,0 +1,20 @@
+/**
+ * @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
+ * @precision medium
+ * @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."

cpp/ql/src/Likely Bugs/Leap Year/Adding365daysPerYear.qhelp

@@ -0,0 +1,20 @@
+<!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>
+  <include src="LeapYearReferences.qhelp" />
+</references>
+</qhelp>

cpp/ql/src/Likely Bugs/Leap Year/Adding365daysPerYear.ql

@@ -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 error
+ * @id cpp/leap-year/adding-365-days-per-year
+ * @precision high
+ * @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()

cpp/ql/src/Likely Bugs/Leap Year/LeapYear.qhelp

@@ -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>

cpp/ql/src/Likely Bugs/Leap Year/LeapYear.qll

@@ -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
+    )
+  }
+}

cpp/ql/src/Likely Bugs/Leap Year/LeapYearReferences.qhelp

@@ -0,0 +1,10 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+<fragment>
+  <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>
+</fragment>
+</qhelp>

cpp/ql/src/Likely Bugs/Leap Year/UncheckedLeapYearAfterYearModification.qhelp

@@ -0,0 +1,27 @@
+<!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>
+  <include src="LeapYearReferences.qhelp" />
+</references>
+</qhelp>

cpp/ql/src/Likely Bugs/Leap Year/UncheckedLeapYearAfterYearModification.ql

@@ -0,0 +1,58 @@
+/**
+ * @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 error
+ * @id cpp/leap-year/unchecked-after-arithmetic-year-modification
+ * @precision high
+ * @tags security
+ *       leap-year
+ */
+
+import cpp
+import LeapYear
+
+from Variable var, LeapYearFieldAccess yfa 
+where
+   exists(VariableAccess va |
+     yfa.getQualifier() = va
+     and var.getAnAccess() = va
+     // The year is modified with an arithmetic operation. Avoid values that are likely false positives
+     and yfa.isModifiedByArithmeticOperationNotForNormalization()
+     // Avoid false positives
+     and 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*()
+        )
+        // If there is a data flow from the variable that was modified to a function that seems to check for leap year 
+        or exists(VariableAccess source,
+          ChecksForLeapYearFunctionCall fc, 
+          LeapYearCheckConfiguration config |
+          source = var.getAnAccess()
+          and config.hasFlow( DataFlow::exprNode(source), DataFlow::exprNode(fc.getAnArgument()))
+       )
+       // If there is a data flow from the field that was modified to a function that seems to check for leap year
+       or 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()))
+       )
+       // If there is a successor or predecessor that sets the month = 1
+       or 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()

cpp/ql/src/Likely Bugs/Leap Year/UncheckedLeapYearAfterYearModificationBad.c

@@ -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);

cpp/ql/src/Likely Bugs/Leap Year/UncheckedLeapYearAfterYearModificationGood.c

@@ -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
+}

cpp/ql/src/Likely Bugs/Leap Year/UncheckedReturnValueForTimeFunctions.qhelp

@@ -0,0 +1,39 @@
+<!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>
+  <list>
+  <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>
+</list>
+
+</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>
+  <include src="LeapYearReferences.qhelp" />
+</references>
+</qhelp>

cpp/ql/src/Likely Bugs/Leap Year/UncheckedReturnValueForTimeFunctions.ql

@@ -0,0 +1,107 @@
+/**
+ * @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 error
+ * @id cpp/leap-year/unchecked-return-value-for-time-conversion-function
+ * @precision high
+ * @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*()
+  )
+  // Remove false positives
+  and 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*()
+    )
+  )
+  // Remove any instance where the year is changed, but the month is set to 1 (year wrapping)
+  or 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()

cpp/ql/src/Likely Bugs/Leap Year/UnsafeArrayForDaysOfYear.qhelp

@@ -0,0 +1,28 @@
+<!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>
+  <include src="LeapYearReferences.qhelp" />
+</references>
+</qhelp>

cpp/ql/src/Likely Bugs/Leap Year/UnsafeArrayForDaysOfYear.ql

@@ -0,0 +1,36 @@
+/**
+ * @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 error
+ * @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."

cpp/ql/src/Likely Bugs/Leap Year/UnsafeArrayForDaysOfYearBad.c

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

cpp/ql/src/Likely Bugs/Leap Year/UnsafeArrayForDaysOfYearGood.c

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

cpp/ql/src/semmle/code/cpp/commons/DateTime.qll

@@ -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") 
+  }
+}