Merge pull request #1345 from denislevin/denisl/cs/MishandlingJapaneseDatesAndLeapYear

C#: Japanese Era and Leap Year checks (Likely Bugs)

Author: calumgrant

csharp/ql/src/Likely Bugs/LeapYear/UnsafeYearConstruction.qhelp

@@ -0,0 +1,23 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+<overview>
+  <p>When creating a <code>System.DateTime</code> object by setting the year, month, and day in the constructor by performing an arithmetic operation on a different <code>DateTime</code> object, there is a risk that the date you are setting is invalid.</p>
+  <p>On a leap year, such code may throw an <code>ArgumentOutOfRangeException</code> with a message of <code>"Year, Month, and Day parameters describe an unrepresentable DateTime."</code></p>
+</overview>
+<recommendation>
+  <p>Creating a <code>System.DateTime</code> object based on a different <code>System.DateTime</code> object, use the appropriate methods to manipulate the date instead of arithmetic.</p>
+</recommendation>
+<example>
+  <p>In this example, we are incrementing/decrementing the current date by one year when creating a new <code>System.DateTime</code> object. This may work most of the time, but on any given February 29th, the resulting value will be invalid.</p>
+  <sample src="UnsafeYearConstructionBad.cs" />
+  <p>To fix this bug, we add/substract years to the current date by calling <code>AddYears</code> method on it.</p>
+  <sample src="UnsafeYearConstructionGood.cs" />
+</example>
+<references>
+  <li>
+    <a href="https://docs.microsoft.com/en-us/dotnet/api/system.datetime?view=netframework-4.8">System.DateTime Struct</a>.
+  </li>
+</references>
+</qhelp>

csharp/ql/src/Likely Bugs/LeapYear/UnsafeYearConstruction.ql

@@ -0,0 +1,39 @@
+/**
+ * @name Unsafe year argument for 'DateTime' constructor
+ * @description Constructing a 'DateTime' struct by setting the year argument to an increment or decrement of the year of a different 'DateTime' struct
+ * @kind path-problem
+ * @problem.severity error
+ * @id cs/unsafe-year-construction
+ * @precision high
+ * @tags security
+ *       date-time
+ *       reliability
+ */
+
+import csharp
+import DataFlow::PathGraph
+import semmle.code.csharp.dataflow.TaintTracking
+
+class UnsafeYearCreationFromArithmeticConfiguration extends TaintTracking::Configuration {
+  UnsafeYearCreationFromArithmeticConfiguration() {
+    this = "UnsafeYearCreationFromArithmeticConfiguration"
+  }
+
+  override predicate isSource(DataFlow::Node source) {
+    exists(ArithmeticOperation ao, PropertyAccess pa | ao = source.asExpr() |
+      pa = ao.getAChild*() and
+      pa.getProperty().getQualifiedName().matches("System.DateTime.Year")
+    )
+  }
+
+  override predicate isSink(DataFlow::Node sink) {
+    exists(ObjectCreation oc |
+      sink.asExpr() = oc.getArgumentForName("year") and
+      oc.getObjectType().getABaseType*().hasQualifiedName("System.DateTime")
+    )
+  }
+}
+
+from UnsafeYearCreationFromArithmeticConfiguration config, DataFlow::PathNode source, DataFlow::PathNode sink
+where config.hasFlowPath(source, sink)
+select sink, source, sink, "This $@ based on a 'System.DateTime.Year' property is used in a construction of a new 'System.DateTime' object, flowing to the 'year' argument.", source, "arithmetic operation"

csharp/ql/src/Likely Bugs/LeapYear/UnsafeYearConstructionBad.cs

@@ -0,0 +1,13 @@
+using System;
+public class UnsafeYearConstructionBad
+{
+    public UnsafeYearConstructionBad()
+    {
+        DateTime Start;
+        DateTime End;
+        var now = DateTime.UtcNow;
+        // the base-date +/- n years may not be a valid date.
+        Start = new DateTime(now.Year - 1, now.Month, now.Day, 0, 0, 0, DateTimeKind.Utc);
+        End = new DateTime(now.Year + 1, now.Month, now.Day, 0, 0, 1, DateTimeKind.Utc);
+    }
+}

csharp/ql/src/Likely Bugs/LeapYear/UnsafeYearConstructionGood.cs

@@ -0,0 +1,12 @@
+using System;
+public class UnsafeYearConstructionGood
+{
+    public UnsafeYearConstructionGood()
+    {
+        DateTime Start;
+        DateTime End;
+        var now = DateTime.UtcNow;
+        Start = now.AddYears(-1).Date;
+        End = now.AddYears(-1).Date.AddSeconds(1);
+    }
+}

csharp/ql/src/Likely Bugs/MishandlingJapaneseEra.cs

@@ -0,0 +1,21 @@
+using  System;
+using System.Globalization;
+public class Example
+{
+   public static void Main()
+   {
+      var cal = new JapaneseCalendar();
+      // constructing date using current era 
+      var dat = cal.ToDateTime(2, 1, 2, 0, 0, 0, 0);
+      Console.WriteLine($"{dat:s}");
+      // constructing date using current era 
+      dat = new DateTime(2, 1, 2, cal);
+      Console.WriteLine($"{dat:s}");
+   }
+}
+// Output with the Heisei era current:
+//      1990-01-02T00:00:00
+//      1990-01-02T00:00:00
+// Output with the new era current:
+//      2020-01-02T00:00:00
+//      2020-01-02T00:00:00

csharp/ql/src/Likely Bugs/MishandlingJapaneseEra.qhelp

@@ -0,0 +1,36 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+<overview>
+  <p>
+  When eras change, calling a date and time instantiation method that relies on the default era can produce an ambiguous date. 
+  In the example below, the call to the <code>JapaneseCalendar.ToDateTime</code> method that uses the default era returns different dates depending on whether or not the new era has been defined in the registry.
+  </p>
+</overview>
+<recommendation>
+  <p>Use speific era when creating DateTime and DateTimeOffset structs from previously stored date in Japanese calendar</p>
+  <p>Don't store dates in Japanese format</p>
+  <p>Don't use hard-coded era start date for date calculations converting dates from Japanese date format</p>
+  <p>Use <code>JapaneseCalendar</code> class for date formatting only</p>
+</recommendation>
+<example>
+  <p>This example demonstrates the dangers of using current year assumptions in Japanese date conversions</p>
+  <sample src="MishandlingJapaneseEra.cs" />
+</example>
+
+<references>
+  <li>
+    <a href="https://devblogs.microsoft.com/dotnet/handling-a-new-era-in-the-japanese-calendar-in-net/">Handling a new era in the Japanese calendar in .NET</a>.
+  </li>
+  <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>
+  <li>
+    <a href="https://docs.microsoft.com/en-us/windows/desktop/Intl/era-handling-for-the-japanese-calendar/">Era Handling for the Japanese Calendar</a>.
+  </li>
+  <li>
+   <a href="https://simple.wikipedia.org/wiki/List_of_Japanese_eras">List of Japanese Eras (Wikipedia)</a>
+  </li>
+</references>
+</qhelp>

csharp/ql/src/Likely Bugs/MishandlingJapaneseEra.ql

@@ -0,0 +1,72 @@
+/**
+ * @name Mishandling the Japanese era start date
+ * @description Japanese era should be handled with the built-in 'JapaneseCalendar' class. Avoid hard-coding Japanese era start dates and names.
+ * @id cs/mishandling-japanese-era
+ * @kind problem
+ * @problem.severity warning
+ * @precision medium
+ * @tags reliability
+ *       date-time
+ */
+
+import csharp
+
+/**
+ * Holds if `year`, `month`, and `day` specify the start of a new era
+ * (see https://simple.wikipedia.org/wiki/List_of_Japanese_eras).
+ */
+predicate isEraStart(int year, int month, int day) {
+  year = 1989 and month = 1 and day = 8
+  or
+  year = 2019 and month = 5 and day = 1
+}
+
+predicate isExactEraStartDateCreation(ObjectCreation cr) {
+  (
+    cr.getType().hasQualifiedName("System.DateTime") or
+    cr.getType().hasQualifiedName("System.DateTimeOffset")
+  ) and
+  isEraStart(cr.getArgument(0).getValue().toInt(), cr.getArgument(1).getValue().toInt(), cr.getArgument(2).getValue().toInt())
+}
+
+predicate isDateFromJapaneseCalendarToDateTime(MethodCall mc) {
+  (
+    mc.getQualifier().getType().hasQualifiedName("System.Globalization.JapaneseCalendar") or
+    mc.getQualifier().getType().hasQualifiedName("System.Globalization.JapaneseLunisolarCalendar")
+  ) and
+  mc.getTarget().hasName("ToDateTime") and
+  mc.getArgument(0).hasValue() and
+  (
+    mc.getNumberOfArguments() = 7 // implicitly current era
+    or
+    mc.getNumberOfArguments() = 8 and
+    mc.getArgument(7).getValue() = "0"
+  ) // explicitly current era
+}
+
+predicate isDateFromJapaneseCalendarCreation(ObjectCreation cr) {
+  (
+    cr.getType().hasQualifiedName("System.DateTime") or
+    cr.getType().hasQualifiedName("System.DateTimeOffset")
+  ) and
+  (
+    cr
+        .getArgumentForName("calendar")
+        .getType()
+        .hasQualifiedName("System.Globalization.JapaneseCalendar") or
+    cr
+        .getArgumentForName("calendar")
+        .getType()
+        .hasQualifiedName("System.Globalization.JapaneseLunisolarCalendar")
+  ) and
+  cr.getArgumentForName("year").hasValue()
+}
+
+from Expr expr, string message
+where
+  isDateFromJapaneseCalendarToDateTime(expr) and message = "'DateTime' created from Japanese calendar with explicit or current era and hard-coded year."
+  or
+  isDateFromJapaneseCalendarCreation(expr) and message = "'DateTime' constructed from Japanese calendar with explicit or current era and hard-coded year."
+  or
+  isExactEraStartDateCreation(expr) and message = "Hard-coded the beginning of the Japanese Heisei era."
+select expr, message

csharp/ql/test/query-tests/Likely Bugs/MishandlingJapaneseEra/MishandlingJapaneseEra.expected

@@ -0,0 +1,6 @@
+| Program.cs:12:31:12:54 | object creation of type DateTime | Hard-coded the beginning of the Japanese Heisei era. |
+| Program.cs:15:66:15:89 | object creation of type DateTime | Hard-coded the beginning of the Japanese Heisei era. |
+| Program.cs:22:42:22:98 | object creation of type DateTimeOffset | Hard-coded the beginning of the Japanese Heisei era. |
+| Program.cs:29:37:29:71 | call to method ToDateTime | 'DateTime' created from Japanese calendar with explicit or current era and hard-coded year. |
+| Program.cs:33:27:33:64 | call to method ToDateTime | 'DateTime' created from Japanese calendar with explicit or current era and hard-coded year. |
+| Program.cs:49:28:49:73 | object creation of type DateTime | 'DateTime' constructed from Japanese calendar with explicit or current era and hard-coded year. |

csharp/ql/test/query-tests/Likely Bugs/MishandlingJapaneseEra/MishandlingJapaneseEra.qlref

@@ -0,0 +1 @@
+Likely Bugs/MishandlingJapaneseEra.ql

csharp/ql/test/query-tests/Likely Bugs/MishandlingJapaneseEra/Program.cs

@@ -0,0 +1,57 @@
+using System;
+using System.Collections.Generic;
+using System.Globalization;
+
+namespace JapaneseDates
+{
+    class Program
+    {
+        static void Main(string[] args)
+        {
+            // BAD: hard-coded era start date
+            var henseiStart = new DateTime(1989, 1, 8);
+
+            // BAD: hard-coded era start dates, list
+            List<DateTime> listOfEraStart = new List<DateTime> { new DateTime(1989, 1, 8) };
+
+            // BAD: hardcoded era name
+            string currentEra = "Heisei";
+
+            DateTimeOffset dateNow = DateTimeOffset.Now;
+
+            DateTimeOffset dateThisEra = new DateTimeOffset(1989, 1, 8, 0, 0, 0, 0, TimeSpan.Zero);
+
+            CultureInfo japaneseCulture = CultureInfo.GetCultureInfo("ja-JP");
+
+            JapaneseCalendar jk = new JapaneseCalendar();
+
+            // BAD: datetime is created from constant year in the current era, and the result will change with era change
+            var datejkCurrentEra  = jk.ToDateTime(32, 2, 1, 9, 9, 9, 9);
+            Console.WriteLine("Date for datejkCurrentEra {0} and year {1}",  datejkCurrentEra.ToString(japaneseCulture), jk.GetYear (datejkCurrentEra));
+
+            // BAD: datetime is created from constant year in the current era, and the result will change with era change
+            var datejk  = jk.ToDateTime(32, 2, 1, 9, 9, 9, 9, 0);
+            Console.WriteLine("Date for jk {0} and year {1}", datejk.ToString(japaneseCulture), jk.GetYear (datejk));
+
+            // OK: datetime is created from constant year in the specific era, and the result will not change with era change
+            var datejk1  = jk.ToDateTime(32, 2, 1, 9, 9, 9, 9, 4);
+            Console.WriteLine("Date for jk {0} and year {1}", datejk1.ToString(japaneseCulture), jk.GetYear (datejk1));
+
+            // OK: year is not hard-coded, i.e. it may be updated
+            var datejk0 = jk.ToDateTime(jk.GetYear(datejk), 2, 1, 9, 9, 9, 9);
+            Console.WriteLine("Date for jk0 {0} and year {1}", datejk0, jk.GetYear(datejk0));
+
+            // BAD: hard-coded year conversion
+            int realYear = 1988 + jk.GetYear(datejk);
+            Console.WriteLine("Which converts to year {0}", realYear);
+
+            // BAD: creating DateTime using specified Japanese era date. This may yield a different date when era changes
+            DateTime val = new DateTime(32, 2, 1, new JapaneseCalendar());
+            Console.WriteLine("DateTime from constructor {0}", val);
+
+            // OK: variable data for Year, not necessarily hard-coded and can come from adjusted source
+            DateTime val1 = new DateTime(jk.GetYear(datejk), 2, 1, new JapaneseCalendar());
+            Console.WriteLine("DateTime from constructor {0}", val);
+        }
+    }
+}