Skip to content

Commit 9d7f644

Browse files
committed
C#: Update the barrier in HashWithoutSalt to avoid an FP. It worked by accident before as we didn't allow implicit element reads at sinks.
1 parent 0210e91 commit 9d7f644

File tree

1 file changed

+12
-6
lines changed

1 file changed

+12
-6
lines changed

csharp/ql/src/experimental/Security Features/CWE-759/HashWithoutSalt.ql

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
*/
1111

1212
import csharp
13+
import semmle.code.csharp.frameworks.system.Collections
1314
import HashWithoutSalt::PathGraph
1415

1516
/** The C# class `Windows.Security.Cryptography.Core.HashAlgorithmProvider`. */
@@ -93,12 +94,17 @@ predicate hasAnotherHashCall(MethodCall mc) {
9394

9495
/** Holds if a password hash without salt is further processed in another method call. */
9596
predicate hasFurtherProcessing(MethodCall mc) {
96-
mc.getTarget().fromLibrary() and
97-
(
98-
mc.getTarget().hasFullyQualifiedName("System", "Array", "Copy") or // Array.Copy(passwordHash, 0, password.Length), 0, key, 0, keyLen);
99-
mc.getTarget().hasFullyQualifiedName("System", "String", "Concat") or // string.Concat(passwordHash, saltkey)
100-
mc.getTarget().hasFullyQualifiedName("System", "Buffer", "BlockCopy") or // Buffer.BlockCopy(passwordHash, 0, allBytes, 0, 20)
101-
mc.getTarget().hasFullyQualifiedName("System", "String", "Format") // String.Format("{0}:{1}:{2}", username, salt, password)
97+
exists(Method m | m = mc.getTarget() and m.fromLibrary() |
98+
m.hasFullyQualifiedName("System", "Array", "Copy") // Array.Copy(passwordHash, 0, password.Length), 0, key, 0, keyLen);
99+
or
100+
m.hasFullyQualifiedName("System", "String", "Concat") // string.Concat(passwordHash, saltkey)
101+
or
102+
m.hasFullyQualifiedName("System", "Buffer", "BlockCopy") // Buffer.BlockCopy(passwordHash, 0, allBytes, 0, 20)
103+
or
104+
m.hasFullyQualifiedName("System", "String", "Format") // String.Format("{0}:{1}:{2}", username, salt, password)
105+
or
106+
m.getName() = "CopyTo" and
107+
m.getDeclaringType().getABaseType*() instanceof SystemCollectionsICollectionInterface // passBytes.CopyTo(rawSalted, 0);
102108
)
103109
}
104110

0 commit comments

Comments
 (0)