Japanese Era and Leap Year checks (Likely Bugs)

Author: Denis Levin

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

@@ -0,0 +1,24 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+<overview>
+  <include src="LeapYear.qhelp" />
+  
+<p>When creating or creating a <code>System.DateTime</code> object by setting the year, month, day in the contructor (other parameters optional) by performing an arithmetic operation on a different <code>DateTime</code> object, there is a risk that the date you are setting is an invalid one.</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 un-representable 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 adding/decreasing 1 year to the current date 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>
+
+</qhelp>

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

@@ -0,0 +1,35 @@
+/**
+ * @name Unsafe year argument for DateTime constructor
+ * @description Constructing a DateTime object by setting the year argument by manipulating the year of a different DateTime object
+ * @kind problem
+ * @problem.severity error
+ * @id cs/leap-year/unsafe-year-contruction
+ * @precision high
+ * @tags security
+ *       leap-year
+ */
+
+import csharp
+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("%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, Expr sink, Expr source
+where config.hasFlow(DataFlow::exprNode(source), DataFlow::exprNode(sink))
+select 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,15 @@
+using System;
+
+public class UnsafeYearConstructionBad
+{
+    public UnsafeYearConstructionBad()
+    {
+        DateTime Start;
+        DateTime End;
+        var now = DateTime.UtcNow;
+
+        // the base-date +/1 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,14 @@
+using System;
+
+public class UnsafeYearConstructionBad
+{
+    public UnsafeYearConstructionBad()
+    {
+        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,39 @@
+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,33 @@
+<!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>
+</references>
+</qhelp>

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

@@ -0,0 +1,60 @@
+/**
+ * @name MishandlingJapaneseEraStartDate
+ * @description Japanese Era should be handled with built-in JapaneseCalendar class. Aviod hard-coding Japanese era start dates and names. 
+ * @id cs/mishandling-japanese-era
+ * @kind problem
+ * @problem.severity warning
+ * @precision medium
+ * @tags reliability
+ *       japanese-era
+ */
+
+import csharp
+
+predicate isExactEraStartDateCreation(ObjectCreation cr) {
+  (cr.getType().hasQualifiedName("System.DateTime") or cr.getType().hasQualifiedName("System.DateTimeOffset"))
+  and  
+    cr.getArgument(0).getValue() = "1989" and
+    cr.getArgument(1).getValue() = "1" and
+    cr.getArgument(2).getValue() = "8"
+}
+
+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).getValue() != "" and
+    (mc.getNumberOfArguments() = 7 or // implicitly current era
+     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").getValue() != ""
+}
+
+predicate inEraArrayCreation(ArrayInitializer ai) {
+  ai.getElement(0).getValue() = "1867" and
+  ai.getElement(1).getValue() = "1911" and
+  ai.getElement(2).getValue() = "1925" and
+  ai.getElement(3).getValue() = "1988"
+}
+
+predicate isEraCollectionCreation(CollectionInitializer cs) {
+  cs.getElementInitializer(0).getValue() = "1867" and
+  cs.getElementInitializer(1).getValue() = "1911" and
+  cs.getElementInitializer(2).getValue() = "1925" and
+  cs.getElementInitializer(3).getValue() = "1988"	
+}
+
+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
+  isEraCollectionCreation(expr) and message = "Hard-coded collection with Japanese era years" or
+  inEraArrayCreation (expr) and message = "Hard-coded array with Japanese era years" or
+  isExactEraStartDateCreation(expr) and message = "Hard-coded the beginning of the Japanese Heisei era"  
+select expr, message

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

@@ -0,0 +1,3 @@
+| Program.cs:13:39:13:50 | ... - ... | This $@ based on a System.DateTime.Year property is used in a construction of a new System.DateTime object, flowing to the 'year' argument. | Program.cs:13:39:13:50 | ... - ... | arithmetic operation |
+| Program.cs:17:37:17:43 | access to local variable endYear | This $@ based on a System.DateTime.Year property is used in a construction of a new System.DateTime object, flowing to the 'year' argument. | Program.cs:15:27:15:38 | ... + ... | arithmetic operation |
+| Program.cs:26:39:26:42 | access to parameter year | This $@ based on a System.DateTime.Year property is used in a construction of a new System.DateTime object, flowing to the 'year' argument. | Program.cs:33:18:33:29 | ... - ... | arithmetic operation |

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

@@ -0,0 +1 @@
+Likely Bugs/LeapYear/UnsafeYearConstruction.ql

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

@@ -0,0 +1,42 @@
+using System;
+
+namespace LeapYear
+{
+    public class PipelineProperties
+    {
+        public DateTime Start;
+        public DateTime End;
+        public PipelineProperties()
+        {
+            var now = DateTime.UtcNow;
+            // BUG
+            this.Start = new DateTime(now.Year - 1, now.Month, now.Day, 0, 0, 0, DateTimeKind.Utc);
+
+            var endYear = now.Year + 1;
+            // BUG
+            this.End = new DateTime(endYear, now.Month, now.Day, 0, 0, 1, DateTimeKind.Utc);
+
+            // OK
+            this.Start = now.AddYears(-1).Date;
+        }
+
+        private void Test(int year, int month, int day)
+        {
+            // BUG (arithmetic operation from StartTest)
+            this.Start = new DateTime(year, month, day);
+        }
+
+        public void StartTest()
+        {
+            var now = DateTime.UtcNow;
+            // flows into Test (source for bug)
+            Test(now.Year - 1, now.Month, now.Day);
+        }
+
+        public void StartTestFP()
+        {
+            var now = DateTime.UtcNow;
+            Test(1900 + 80, now.Month, now.Day);
+        }
+    }
+}