Determine whether the time span in question contains a leap day, then perform the calculation using the correct number -of days. Alternatively, use an established library routine that already contains correct leap year logic.
+of days. Alternatively, use an established library routine that already contains correct leap year logic.This anti-pattern occurs when a developer uses conditional logic to execute a different path of code for a leap year than for a common year, without fully testing both code paths.
+Though using a framework or library's leap year function is better than manually calculating the leap year (as described in anti-pattern 5), it can still be a source of errors if the result is used to execute a different code path. The bug is especially easy to be masked if the year is derived from the current time of the system clock. See Prevention Measures for techniques to avoid this bug.
+Note in the following examples, that year, month, and day might instead be .wYear, .wMonth, and .wDay fields of a SYSTEMTIME structure, or might be .tm_year, .tm_mon, and .tm_mday fields of a struct tm.
+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.
-To fix this bug, check the result for leap year.
-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.
-To fix this bug, you must verify the return value for SystemTimeToFileTime and handle any potential error accordingly.
In this example, we are allocating 365 integers, one for each day of the year. This code will fail on a leap year, when there are 366 days.
-When using arrays, allocate the correct number of elements to match the year.
-Checking for overflow of an addition by comparing against one of the arguments of the addition fails if the size of all the argument types are smaller than 4 bytes. This is because the result of the addition is promoted to a 4 byte int.
+Check the overflow by comparing the addition against a value that is at least 4 bytes.
+In this example, the result of the comparison will result in an integer overflow.
+To fix the bug, check the overflow by comparing the addition against a value that is at least 4 bytes.
+RtlCompareMemory routine compares two blocks of memory and returns the number of bytes that match, not a boolean value indicating a full comparison like RtlEqualMemory does.
This query detects the return value of RtlCompareMemory being handled as a boolean.
Any findings from this rule may indicate that the return value of a call to RtlCompareMemory is being incorrectly interpreted as a boolean.
Review the logic of the call, and if necessary, replace the function call with RtlEqualMemory.
The following example is a typical one where an identity comparison is intended, but the wrong API is being used.
+In this example, the fix is to replace the call to RtlCompareMemory with RtlEqualMemory.
If the argument for a sizeof call is a binary operation or a sizeof call, it is typically a sign that there is a confusion on the usage of the sizeof usage.
Any findings from this rule may indicate that the sizeof is being used incorrectly.
Review the logic of the call.
+The following example shows a case where sizeof a binary operation by mistake.
In this example, the fix is to multiply the result of sizeof by the number of elements.
If the argument for a sizeof call is a macro that expands to a constant integer type, it is a likely indication that the macro operation may be misused or that the argument was selected by mistake (i.e. typo).
This query detects if the argument for sizeof is a macro that expands to a constant integer value.
NOTE: This rule will ignore multicharacter literal values that are exactly 4 bytes long as it matches the length of int and may be expected.
Any findings from this rule may indicate that the sizeof is being used incorrectly.
Review the logic of the call.
+The following example shows a case where sizeof a constant was used instead of the sizeof of a structure by mistake as the names are similar.
In this example, the fix is to replace the argument for sizeof with the structure name.
+ Finds explicit uses of symmetric encryption algorithms that are weak, obsolete, or otherwise unapproved. +
++ Encryption algorithms such as DES, (uses keys of 56 bits only), RC2 (uses keys of 128 bits only), and TripleDES (provides at most 112 bits of security) are considered to be weak. +
++ These cryptographic algorithms do not provide as much security assurance as more modern counterparts. +
++ For Microsoft internal security standards: +
++ For WinCrypt, switch to ALG_SID_AES, ALG_SID_AES_128, ALG_SID_AES_192, or ALG_SID_AES_256. +
++ For BCrypt, switch to AES or any algorithm other than RC2, RC4, DES, DESX, 3DES, 3DES_112. AES_GMAC and AES_CMAC require crypto board review. +
+Violations:
+ +Solutions:
+ +Violation - Use of one of the following unsafe encryption modes that is not approved: ECB, OFB, CFB, CTR, CCM, or GCM.
+ These modes are vulnerable to attacks and may cause exposure of sensitive information. For example, using ECB to encrypt a plaintext block always produces a same cipher text, so it can easily tell if two encrypted messages are identical. Using approved modes can avoid these unnecessary risks.
- Use only approved modes CBC, CTS and XTS.
+Violation:
+ +Solution:
+ +Violation - Use of one of the following unsafe encryption modes that is not approved: ECB, OFB, CFB, CTR, CCM, or GCM.
+ These modes are vulnerable to attacks and may cause exposure of sensitive information. For example, using ECB to encrypt a plaintext block always produces a same cipher text, so it can easily tell if two encrypted messages are identical. Using approved modes can avoid these unnecessary risks.
- Use only approved modes CBC, CTS and XTS.
+Violation:
+ +Solution:
+ +An initialization vector (IV) is an input to a cryptographic primitive being used to provide the initial state. The IV is typically required to be random or pseudorandom (randomized scheme), but sometimes an IV only needs to be unpredictable or unique (stateful scheme).
+Randomization is crucial for some encryption schemes to achieve semantic security, a property whereby repeated usage of the scheme under the same key does not allow an attacker to infer relationships between (potentially similar) segments of the encrypted message.
+All symmetric block ciphers must also be used with an appropriate initialization vector (IV) according to the mode of operation being used.
+If using a randomized scheme such as CBC, it is recommended to use cryptographically secure pseudorandom number generator such as BCryptGenRandom.
Use of KDF algorithm BCryptDeriveKeyPBKDF2 uses insecure hash from BCryptOpenAlgorithmProvider.
+Use SHA 256, 384, or 512.
+Use of KDF algorithm BCryptDeriveKeyPBKDF2 uses low iteration count (less than 100k).
+Use a minimum of 100,000 for iteration count.
+Use of KDF algorithm BCryptDeriveKeyPBKDF2 uses small key size (less than 16 bytes).
+Use a minimum of 16 bytes for key size.
+Use of KDF algorithm BCryptDeriveKeyPBKDF2 uses small salt size (less than 16 bytes).
+Use a minimum of 16 bytes for salt size.
+This rule finds code where an enumerated type (enum) is used to check for an upper boundary, but not the lower boundary, and the value is used as an index to access an array.
By default an enum variable is signed, and therefore it is important to ensure that it cannot take on a negative value. When the enum is subsequently used to index an array, or worse still an array of function pointers, then a negative enum value would lead to potentially arbitrary memory being read, used and/or executed.
+In the majority of cases the fix is simply to add the required lower bounds check to ensure that the enum has a positive value.
+The following example a value is passed and gets cast to an enumerated type and only partially bounds checked.
+In this example, the result of the out-of-bounds may allow for arbitrary code execution.
+To fix the problem in this example, you need to add an additional check to the guarding if statement to verify that the index is a positive value.
+Hard-coding security protocols rather than specifying the system default is risky because the protocol may become deprecated in future.
+The grbitEnabledProtocols member of the SCHANNEL_CRED struct contains a bit string that represents the protocols supported by connections made with credentials acquired by using this structure. If this member is zero, Schannel selects the protocol. Applications should set grbitEnabledProtocols to zero and use the protocol versions enabled on the system by default.
- Set the grbitEnabledProtocols member of the SCHANNEL_CRED struct to 0.
Violation:
+ +Solution:
+ +Older protocol versions of TLS are less secure than TLS 1.2 and TLS 1.3 and are more likely to have new vulnerabilities. Avoid older protocol versions to minimize risk.
+The grbitEnabledProtocols member of the SCHANNEL_CRED struct contains a bit string that represents the protocols supported by connections made with credentials acquired by using this structure. If this member is zero, Schannel selects the protocol. Applications should set grbitEnabledProtocols to zero and use the protocol versions enabled on the system by default.
- Set the grbitEnabledProtocols member of the SCHANNEL_CRED struct to 0.
Violation:
+ +Solution:
+ +c:\sneaky-file.
Ensure that output paths constructed from zip archive entries are validated to prevent writing files to unexpected locations.
-The recommended way of writing an output file from a zip archive entry is to:
+The recommended way of writing an output file from a zip archive entry is to conduct the following in sequence:
Path.Combine(destinationDirectory, archiveEntry.FullName) to determine the raw
diff --git a/csharp/ql/src/Security Features/CWE-327/InsecureSQLConnection.ql b/csharp/ql/src/Security Features/CWE-327/InsecureSQLConnection.ql
index adde1b948e4f..e5fc7e720925 100644
--- a/csharp/ql/src/Security Features/CWE-327/InsecureSQLConnection.ql
+++ b/csharp/ql/src/Security Features/CWE-327/InsecureSQLConnection.ql
@@ -12,34 +12,57 @@
import csharp
import InsecureSqlConnection::PathGraph
+import InsecureSQLConnection
-/**
- * A data flow configuration for tracking strings passed to `SqlConnection[StringBuilder]` instances.
- */
-module InsecureSqlConnectionConfig implements DataFlow::ConfigSig {
- predicate isSource(DataFlow::Node source) {
- exists(string s | s = source.asExpr().(StringLiteral).getValue().toLowerCase() |
- s.matches("%encrypt=false%")
- or
- not s.matches("%encrypt=%")
+class Source extends DataFlow::Node {
+ string sourcestring;
+
+ Source() {
+ sourcestring = this.asExpr().(StringLiteral).getValue().toLowerCase() and
+ (
+ not sourcestring.matches("%encrypt=%") or
+ sourcestring.matches("%encrypt=false%")
)
}
- predicate isSink(DataFlow::Node sink) {
+ predicate setsEncryptFalse() { sourcestring.matches("%encrypt=false%") }
+}
+
+class Sink extends DataFlow::Node {
+ Version version;
+
+ Sink() {
exists(ObjectCreation oc |
- oc.getRuntimeArgument(0) = sink.asExpr() and
+ oc.getRuntimeArgument(0) = this.asExpr() and
(
oc.getType().getName() = "SqlConnectionStringBuilder"
or
oc.getType().getName() = "SqlConnection"
) and
- not exists(MemberInitializer mi |
- mi = oc.getInitializer().(ObjectInitializer).getAMemberInitializer() and
- mi.getLValue().(PropertyAccess).getTarget().getName() = "Encrypt" and
- mi.getRValue().(BoolLiteral).getValue() = "true"
- )
+ version = oc.getType().getALocation().(Assembly).getVersion()
)
}
+
+ predicate isEncryptedByDefault() { version.compareTo("4.0") >= 0 }
+}
+
+predicate isEncryptTrue(Source source, Sink sink) {
+ sink.isEncryptedByDefault() and
+ not source.setsEncryptFalse()
+ or
+ exists(ObjectCreation oc, Expr e | oc.getRuntimeArgument(0) = sink.asExpr() |
+ getInfoForInitializedConnEncryption(oc, e) and
+ e.getValue().toLowerCase() = "true"
+ )
+}
+
+/**
+ * A data flow configuration for tracking strings passed to `SqlConnection[StringBuilder]` instances.
+ */
+module InsecureSqlConnectionConfig implements DataFlow::ConfigSig {
+ predicate isSource(DataFlow::Node source) { source instanceof Source }
+
+ predicate isSink(DataFlow::Node sink) { sink instanceof Sink }
}
/**
@@ -48,7 +71,9 @@ module InsecureSqlConnectionConfig implements DataFlow::ConfigSig {
module InsecureSqlConnection = DataFlow::Global