CodeQL queries 1.25
Skip to end of metadata
Go to start of metadata

Name: Unchecked return value for time conversion function

Description: When the return value of a fallible time conversion function is not checked for failure, its output parameters may contain invalid dates.

ID: cpp/leap-year/unchecked-return-value-for-time-conversion-function

Kind: problem

Severity: warning

Precision: medium

Query: UncheckedReturnValueForTimeFunctions.ql
/**
 * @name Unchecked return value for time conversion function
 * @description When the return value of a fallible time conversion function is
 *              not checked for failure, its output parameters may contain
 *              invalid dates.
 * @kind problem
 * @problem.severity warning
 * @id cpp/leap-year/unchecked-return-value-for-time-conversion-function
 * @precision medium
 * @tags leap-year
 *       correctness
 */

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:
 *  - change `extends LeapYearFieldAccess` to `extends FieldAccess`.
 *  - change `this.isModifiedByArithmeticOperation()` to `this.isModified()`.
 * 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 UnpackedTimeType 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() = "GetFileTime" or
    this.getQualifiedName() = "GetSystemTime" or
    this.getQualifiedName() = "NtQuerySystemTime"
  }
}

/**
 * This list of APIs should check for the return value to detect problems during the conversion.
 */
class TimeConversionFunction extends Function {
  TimeConversionFunction() {
    this.getQualifiedName() = "FileTimeToSystemTime" or
    this.getQualifiedName() = "SystemTimeToFileTime" or
    this.getQualifiedName() = "SystemTimeToTzSpecificLocalTime" or
    this.getQualifiedName() = "SystemTimeToTzSpecificLocalTimeEx" or
    this.getQualifiedName() = "TzSpecificLocalTimeToSystemTime" or
    this.getQualifiedName() = "TzSpecificLocalTimeToSystemTimeEx" or
    this.getQualifiedName() = "RtlLocalTimeToSystemTime" or
    this.getQualifiedName() = "RtlTimeToSecondsSince1970" or
    this.getQualifiedName() = "_mkgmtime"
  }
}

from FunctionCall fcall, TimeConversionFunction trf, Variable var
where
  fcall = trf.getACallToThisFunction() and
  fcall instanceof ExprInVoidContext and
  var.getUnderlyingType() instanceof UnpackedTimeType 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()

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.

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.

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.

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.

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.

The following is a list of the functions that this query covers:

  • FileTimeToSystemTime
  • SystemTimeToFileTime
  • SystemTimeToTzSpecificLocalTime
  • SystemTimeToTzSpecificLocalTimeEx
  • TzSpecificLocalTimeToSystemTime
  • TzSpecificLocalTimeToSystemTimeEx
  • RtlLocalTimeToSystemTime
  • RtlTimeToSecondsSince1970
  • _mkgmtime
Recommendation

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.

Example

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.

SYSTEMTIME st;
FILETIME ft;
GetSystemTime(&st);

// Flawed logic may result in invalid date
st.wYear++;

// The following code may fail
SystemTimeToFileTime(&st, &ft);

To fix this bug, you must verify the return value for SystemTimeToFileTime and handle any potential error accordingly.

SYSTEMTIME st;
FILETIME ft;
GetSystemTime(&st);

// Flawed logic may 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
}

References