Merge pull request github#16946 from hvitved/csharp/fewer-version-regexps

C#: Perform fewer `regexpCapture`s when matching version numbers

Author: hvitved

csharp/ql/lib/semmle/code/csharp/Location.qll

@@ -110,35 +110,89 @@ class SourceLocation extends Location, @location_default {
 
 bindingset[version]
 private int versionField(string version, int field) {
-  exists(string format |
-    format = "(\\d+)\\.(\\d+)\\.(\\d+)\\.(\\d+)" or
-    format = "(\\d+)\\.(\\d+)\\.(\\d+)" or
-    format = "(\\d+)\\.(\\d+)"
+  exists(string format, int i |
+    format = "(\\d+)\\.(\\d+)\\.(\\d+)\\.(\\d+)|" + "(\\d+)\\.(\\d+)\\.(\\d+)|" + "(\\d+)\\.(\\d+)" and
+    result = version.regexpCapture(format, i).toInt()
   |
-    result = version.regexpCapture(format, field).toInt()
+    i = [1, 5, 8] and
+    field = 1
+    or
+    i = [2, 6, 9] and
+    field = 2
+    or
+    i = [3, 7] and
+    field = 3
+    or
+    i = 4 and
+    field = 4
   ) and
   result >= 0 and
   result <= 255
 }
 
 /** An assembly version, for example `4.0.0.0` or `4.5`. */
 class Version extends string {
+  private int major;
+
   bindingset[this]
-  Version() { exists(versionField(this, 1)) }
+  Version() { major = versionField(this, 1) }
+
+  bindingset[this]
+  private int getVersionField(int field) {
+    field = 1 and
+    result = major
+    or
+    field in [2 .. 4] and
+    result = versionField(this, field)
+  }
 
   /**
    * Gets field `field` of this version.
    * If the field is unspecified in the version string, then the result is `0`.
    */
   bindingset[this]
   int getField(int field) {
-    field in [1 .. 4] and
-    if exists(versionField(this, field)) then result = versionField(this, field) else result = 0
+    result = this.getVersionField(field)
+    or
+    field in [2 .. 4] and
+    not exists(this.getVersionField(field)) and
+    result = 0
+  }
+
+  bindingset[this]
+  private string getCanonicalizedField(int field) {
+    exists(string s, int length |
+      s = this.getVersionField(field).toString() and
+      length = s.length()
+    |
+      // make each field consist of 3 digits
+      result = concat(int i | i in [1 .. 3 - length] | "0") + s
+    )
+  }
+
+  /**
+   * Gets a canonicalized version of this string, where lexicographical ordering
+   * corresponds to version ordering.
+   */
+  bindingset[this]
+  string getCanonicalizedVersion() {
+    exists(string res, int length |
+      res =
+        strictconcat(int field, string s |
+          s = this.getCanonicalizedField(field)
+        |
+          s, "." order by field
+        ) and
+      length = res.length()
+    |
+      // make each canonicalized version consist of 4 chunks of 3 digits separated by a dot
+      result = res + concat(int i | i = [1 .. 15 - length] / 4 and i > 0 | ".000")
+    )
   }
 
   /** Gets the major version, for example `1` in `1.2.3.4`. */
   bindingset[this]
-  int getMajor() { result = this.getField(1) }
+  int getMajor() { result = major }
 
   /** Gets the major revision, for example `2` in `1.2.3.4`. */
   bindingset[this]
@@ -164,9 +218,7 @@ class Version extends string {
    */
   bindingset[this, other]
   predicate isEarlierThan(Version other) {
-    exists(int i | this.getField(i) < other.getField(i) |
-      forall(int j | j in [1 .. i - 1] | this.getField(j) = other.getField(j))
-    )
+    this.getCanonicalizedVersion() < other.getCanonicalizedVersion()
   }
 
   /**

csharp/ql/lib/semmle/code/csharp/security/xml/InsecureXMLQuery.qll

@@ -5,6 +5,19 @@
 import csharp
 private import semmle.code.csharp.commons.TargetFramework
 
+pragma[nomagic]
+private float getAssemblyVersion(Assembly a) {
+  result = a.getVersion().regexpCapture("([0-9]+\\.[0-9]+).*", 1).toFloat() and
+  // This method is only accurate when we're looking at versions before 4.0.
+  result < 4.0
+}
+
+pragma[nomagic]
+private Version getTargetFrameworkVersion(TargetFrameworkAttribute tfa) {
+  tfa.isNetFramework() and
+  result = tfa.getFrameworkVersion()
+}
+
 /**
  * Holds if the type `t` is in an assembly that has been compiled against a .NET framework version
  * before the given version.
@@ -14,21 +27,16 @@ private predicate isNetFrameworkBefore(Type t, string version) {
   // For assemblies compiled against framework versions before 4 the TargetFrameworkAttribute
   // will not be present. In this case, we can revert back to the assembly version, which may not
   // contain full minor version information.
-  exists(string assemblyVersion |
-    assemblyVersion =
-      t.getALocation().(Assembly).getVersion().regexpCapture("([0-9]+\\.[0-9]+).*", 1)
-  |
-    assemblyVersion.toFloat() < version.toFloat() and
-    // This method is only accurate when we're looking at versions before 4.0.
-    assemblyVersion.toFloat() < 4.0
+  exists(float assemblyVersion |
+    assemblyVersion = getAssemblyVersion(t.getALocation()) and
+    assemblyVersion < version.toFloat()
   )
   or
   // For 4.0 and above the TargetFrameworkAttribute should be present to provide detailed version
   // information.
   exists(TargetFrameworkAttribute tfa |
     tfa.hasElement(t) and
-    tfa.isNetFramework() and
-    tfa.getFrameworkVersion().isEarlierThan(version)
+    getTargetFrameworkVersion(tfa).isEarlierThan(version)
   )
 }
 
@@ -173,7 +181,7 @@ module XmlReader {
       reason = "DTD processing is enabled by default in versions < 4.0" and
       evidence = this and
       not exists(this.getSettings()) and
-      isNetFrameworkBefore(this.(MethodCall).getTarget().getDeclaringType(), "4.0")
+      isNetFrameworkBefore(this.getTarget().getDeclaringType(), "4.0")
       or
       // bad settings flow here
       exists(ObjectCreation settings |