Skip to content

Commit

Permalink
C#: Perform fewer regexpCaptures when matching version numbers
Browse files Browse the repository at this point in the history
  • Loading branch information
hvitved committed Jul 10, 2024
1 parent f183382 commit bcaab32
Show file tree
Hide file tree
Showing 2 changed files with 74 additions and 21 deletions.
67 changes: 56 additions & 11 deletions csharp/ql/lib/semmle/code/csharp/Location.qll
Original file line number Diff line number Diff line change
Expand Up @@ -110,30 +110,77 @@ 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() { major = versionField(this, 1) }

bindingset[this]
Version() { exists(versionField(this, 1)) }
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()
|
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]
private string getCanonicalizedVersion() {
result =
strictconcat(int field, string s |
s = this.getCanonicalizedField(field)
|
s, "." order by field
)
}

/** Gets the major version, for example `1` in `1.2.3.4`. */
Expand Down Expand Up @@ -164,9 +211,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()
}

/**
Expand Down
28 changes: 18 additions & 10 deletions csharp/ql/lib/semmle/code/csharp/security/xml/InsecureXMLQuery.qll
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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)
)
}

Expand Down Expand Up @@ -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 |
Expand Down

0 comments on commit bcaab32

Please sign in to comment.