Merge pull request #4949 from luchua-bc/cs/hash-without-salt

C#: Query to detect hash without salt
This commit is contained in:
Tom Hvitved
2022-01-25 09:04:23 +01:00
committed by GitHub
8 changed files with 615 additions and 0 deletions

View File

@@ -0,0 +1,65 @@
public class Test
{
private const int SaltSize = 32;
// BAD - Hash without a salt.
public static String HashPassword(string password, string strAlgName ="SHA256")
{
IBuffer passBuff = CryptographicBuffer.ConvertStringToBinary(password, BinaryStringEncoding.Utf8);
HashAlgorithmProvider algProvider = HashAlgorithmProvider.OpenAlgorithm(strAlgName);
IBuffer hashBuff = algProvider.HashData(passBuff);
return CryptographicBuffer.EncodeToBase64String(hashBuff);
}
// GOOD - Hash with a salt.
public static string HashPassword2(string password, string salt, string strAlgName ="SHA256")
{
// Concatenate the salt with the password.
IBuffer passBuff = CryptographicBuffer.ConvertStringToBinary(password+salt, BinaryStringEncoding.Utf8);
HashAlgorithmProvider algProvider = HashAlgorithmProvider.OpenAlgorithm(strAlgName);
IBuffer hashBuff = algProvider.HashData(passBuff);
return CryptographicBuffer.EncodeToBase64String(hashBuff);
}
// BAD - Hash without a salt.
public static string HashPassword(string password)
{
SHA256 sha256Hash = SHA256.Create();
byte[] passBytes = System.Text.Encoding.ASCII.GetBytes(password);
byte[] hashBytes = sha256Hash.ComputeHash(passBytes);
return Convert.ToBase64String(hashBytes);
}
// GOOD - Hash with a salt.
public static string HashPassword2(string password)
{
byte[] passBytes = System.Text.Encoding.ASCII.GetBytes(password);
byte[] saltBytes = GenerateSalt();
// Add the salt to the hash.
byte[] rawSalted = new byte[passBytes.Length + saltBytes.Length];
passBytes.CopyTo(rawSalted, 0);
saltBytes.CopyTo(rawSalted, passBytes.Length);
//Create the salted hash.
SHA256 sha256 = SHA256.Create();
byte[] saltedPassBytes = sha256.ComputeHash(rawSalted);
// Add the salt value to the salted hash.
byte[] dbPassword = new byte[saltedPassBytes.Length + saltBytes.Length];
saltedPassBytes.CopyTo(dbPassword, 0);
saltBytes.CopyTo(dbPassword, saltedPassBytes.Length);
return Convert.ToBase64String(dbPassword);
}
public static byte[] GenerateSalt()
{
using (var rng = new RNGCryptoServiceProvider())
{
var randomNumber = new byte[SaltSize];
rng.GetBytes(randomNumber);
return randomNumber;
}
}
}

View File

@@ -0,0 +1,29 @@
<!DOCTYPE qhelp PUBLIC "-//Semmle//qhelp//EN" "qhelp.dtd">
<qhelp>
<overview>
<p>In cryptography, a salt is some random data used as an additional input to a one-way function that hashes a password or pass-phrase. It makes dictionary attacks more difficult.</p>
<p>Without a salt, it is much easier for attackers to pre-compute the hash value using dictionary attack techniques such as rainbow tables to crack passwords.</p>
</overview>
<recommendation>
<p>Use a long random salt of at least 32 bytes then use the combination of password and salt to hash a password or password phrase.</p>
</recommendation>
<example>
<p>The following example shows two ways of hashing. In the 'BAD' cases, no salt is provided. In the 'GOOD' cases, a salt is provided.</p>
<sample src="HashWithoutSalt.cs" />
</example>
<references>
<li>
DZone:
<a href="https://dzone.com/articles/a-look-at-java-cryptography">A Look at Java Cryptography</a>
</li>
<li>
CWE:
<a href="https://cwe.mitre.org/data/definitions/759.html">CWE-759: Use of a One-Way Hash without a Salt</a>
</li>
</references>
</qhelp>

View File

@@ -0,0 +1,200 @@
/**
* @name Use of a hash function without a salt
* @description Hashed passwords without a salt are vulnerable to dictionary attacks.
* @kind path-problem
* @problem.severity error
* @id cs/hash-without-salt
* @tags security
* external/cwe-759
*/
import csharp
import semmle.code.csharp.dataflow.DataFlow2
import semmle.code.csharp.dataflow.TaintTracking
import semmle.code.csharp.dataflow.TaintTracking2
import DataFlow::PathGraph
/** The C# class `Windows.Security.Cryptography.Core.HashAlgorithmProvider`. */
class HashAlgorithmProvider extends RefType {
HashAlgorithmProvider() {
this.hasQualifiedName("Windows.Security.Cryptography.Core", "HashAlgorithmProvider")
}
}
/** The C# class `System.Security.Cryptography.HashAlgorithm`. */
class HashAlgorithm extends RefType {
HashAlgorithm() { this.hasQualifiedName("System.Security.Cryptography", "HashAlgorithm") }
}
/** The C# class `System.Security.Cryptography.KeyedHashAlgorithm`. */
class KeyedHashAlgorithm extends RefType {
KeyedHashAlgorithm() {
this.hasQualifiedName("System.Security.Cryptography", "KeyedHashAlgorithm")
}
}
/**
* The method `ComputeHash()`, `ComputeHashAsync`, `TryComputeHash`, `HashData`, or
* `TryHashData` declared in `System.Security.Cryptography.HashAlgorithm` and the method
* `HashData()` declared in `Windows.Security.Cryptography.Core.HashAlgorithmProvider`.
*/
class HashMethod extends Method {
HashMethod() {
this.getDeclaringType().getABaseType*() instanceof HashAlgorithm and
this.getName().matches(["%ComputeHash%", "%HashData"])
or
this.getDeclaringType().getABaseType*() instanceof HashAlgorithmProvider and
this.hasName("HashData")
}
}
/**
* Gets a regular expression for matching common names of variables that indicate the
* value being held is a password.
*/
string getPasswordRegex() { result = "(?i)pass(wd|word|code|phrase)" }
/** Finds variables that hold password information judging by their names. */
class PasswordVarExpr extends Expr {
PasswordVarExpr() {
exists(Variable v | this = v.getAnAccess() | v.getName().regexpMatch(getPasswordRegex()))
}
}
/**
* Holds if `mc` is a hashing method call or invokes a hashing method call
* directly or indirectly.
*/
predicate isHashCall(MethodCall mc) {
mc.getTarget() instanceof HashMethod
or
exists(MethodCall mcc |
mc.getTarget().calls(mcc.getTarget()) and
isHashCall(mcc) and
DataFlow::localExprFlow(mc.getTarget().getAParameter().getAnAccess(), mcc.getAnArgument())
)
}
/** Holds if there is another hashing method call. */
predicate hasAnotherHashCall(MethodCall mc) {
exists(MethodCall mc2, DataFlow2::Node src, DataFlow2::Node sink |
isHashCall(mc2) and
mc2 != mc and
(
src.asExpr() = mc.getQualifier() or
src.asExpr() = mc.getAnArgument() or
src.asExpr() = mc
) and
(
sink.asExpr() = mc2.getQualifier() or
sink.asExpr() = mc2.getAnArgument()
) and
DataFlow::localFlow(src, sink)
)
}
/** Holds if a password hash without salt is further processed in another method call. */
predicate hasFurtherProcessing(MethodCall mc) {
mc.getTarget().fromLibrary() and
(
mc.getTarget().hasQualifiedName("System.Array", "Copy") or // Array.Copy(passwordHash, 0, password.Length), 0, key, 0, keyLen);
mc.getTarget().hasQualifiedName("System.String", "Concat") or // string.Concat(passwordHash, saltkey)
mc.getTarget().hasQualifiedName("System.Buffer", "BlockCopy") or // Buffer.BlockCopy(passwordHash, 0, allBytes, 0, 20)
mc.getTarget().hasQualifiedName("System.String", "Format") // String.Format("{0}:{1}:{2}", username, salt, password)
)
}
/**
* Holds if `mc` is part of a call graph that satisfies `isHashCall` but is not at the
* top of the call hierarchy.
*/
predicate hasHashAncestor(MethodCall mc) {
exists(MethodCall mpc |
mpc.getTarget().calls(mc.getTarget()) and
isHashCall(mpc) and
DataFlow::localExprFlow(mpc.getTarget().getAParameter().getAnAccess(), mc.getAnArgument())
)
}
/**
* Taint configuration tracking flow from an expression whose name suggests it holds
* password data to a method call that generates a hash without a salt.
*/
class HashWithoutSaltConfiguration extends TaintTracking::Configuration {
HashWithoutSaltConfiguration() { this = "HashWithoutSaltConfiguration" }
override predicate isSource(DataFlow::Node source) { source.asExpr() instanceof PasswordVarExpr }
override predicate isSink(DataFlow::Node sink) {
exists(MethodCall mc |
sink.asExpr() = mc.getArgument(0) and
isHashCall(mc) and
not hasAnotherHashCall(mc) and
not hasHashAncestor(mc) and
not exists(MethodCall mmc |
hasFurtherProcessing(mmc) and
DataFlow::localExprFlow(mc, mmc.getAnArgument())
) and
not exists(Call c |
(
c.getTarget().getDeclaringType().getABaseType*() instanceof HashAlgorithm or
c.getTarget()
.getDeclaringType()
.getABaseType*()
.hasQualifiedName("System.Security.Cryptography", "DeriveBytes")
) and
DataFlow::localExprFlow(mc, c.getAnArgument())
)
)
}
override predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) {
exists(MethodCall mc |
mc.getTarget()
.hasQualifiedName("Windows.Security.Cryptography.CryptographicBuffer",
"ConvertStringToBinary") and
mc.getArgument(0) = node1.asExpr() and
mc = node2.asExpr()
)
}
/**
* Holds if a password is concatenated with a salt then hashed together through calls such as `System.Array.CopyTo()`, for example,
* `byte[] rawSalted = new byte[passBytes.Length + salt.Length];`
* `passBytes.CopyTo(rawSalted, 0);`
* `salt.CopyTo(rawSalted, passBytes.Length);`
* `byte[] saltedPassword = sha256.ComputeHash(rawSalted);`
* Or the password is concatenated with a salt as a string.
*/
override predicate isSanitizer(DataFlow::Node node) {
exists(MethodCall mc |
hasFurtherProcessing(mc) and
mc.getAnArgument() = node.asExpr()
)
or
exists(AddExpr e | node.asExpr() = e.getAnOperand()) // password+salt
or
exists(InterpolatedStringExpr e | node.asExpr() = e.getAnInsert())
or
exists(Call c |
c.getTarget()
.getDeclaringType()
.getABaseType*()
.hasQualifiedName("System.Security.Cryptography", "DeriveBytes")
)
or
// a salt or key is included in subclasses of `KeyedHashAlgorithm`
exists(MethodCall mc, Assignment a, ObjectCreation oc |
a.getRValue() = oc and
oc.getObjectType().getABaseType+() instanceof KeyedHashAlgorithm and
mc.getTarget() instanceof HashMethod and
a.getLValue() = mc.getQualifier().(VariableAccess).getTarget().getAnAccess() and
mc.getArgument(0) = node.asExpr()
)
}
}
from DataFlow::PathNode source, DataFlow::PathNode sink, HashWithoutSaltConfiguration c
where c.hasFlowPath(source, sink)
select sink.getNode(), source, sink, "$@ is hashed without a salt.", source.getNode(),
"The password"

View File

@@ -0,0 +1,202 @@
// semmle-extractor-options: /r:System.Security.Cryptography.Primitives.dll /r:System.Security.Cryptography.Csp.dll /r:System.Security.Cryptography.Algorithms.dll
using System;
using System.Text;
using System.Security.Cryptography;
using Windows.Security.Cryptography;
using Windows.Security.Cryptography.Core;
using Windows.Storage.Streams;
public class Test
{
private const int SaltSize = 32;
// BAD - Hash without a salt.
public static String HashPassword(string password, string strAlgName ="SHA256")
{
IBuffer passBuff = CryptographicBuffer.ConvertStringToBinary(password, BinaryStringEncoding.Utf8);
HashAlgorithmProvider algProvider = HashAlgorithmProvider.OpenAlgorithm(strAlgName);
IBuffer hashBuff = algProvider.HashData(passBuff);
return CryptographicBuffer.EncodeToBase64String(hashBuff);
}
// GOOD - Hash with a salt.
public static string HashPassword2(string password, string salt, string strAlgName ="SHA256")
{
// Concatenate the salt with the password.
IBuffer passBuff = CryptographicBuffer.ConvertStringToBinary(password+salt, BinaryStringEncoding.Utf8);
HashAlgorithmProvider algProvider = HashAlgorithmProvider.OpenAlgorithm(strAlgName);
IBuffer hashBuff = algProvider.HashData(passBuff);
return CryptographicBuffer.EncodeToBase64String(hashBuff);
}
// BAD - Hash without a salt.
public static string HashPassword(string password)
{
SHA256 sha256Hash = SHA256.Create();
byte[] passBytes = System.Text.Encoding.ASCII.GetBytes(password);
byte[] hashBytes = sha256Hash.ComputeHash(passBytes);
return Convert.ToBase64String(hashBytes);
}
// GOOD - Hash with a salt.
public static string HashPassword2(string password)
{
byte[] passBytes = System.Text.Encoding.ASCII.GetBytes(password);
byte[] saltBytes = GenerateSalt();
// Add the salt to the hash.
byte[] rawSalted = new byte[passBytes.Length + saltBytes.Length];
passBytes.CopyTo(rawSalted, 0);
saltBytes.CopyTo(rawSalted, passBytes.Length);
//Create the salted hash.
SHA256 sha256 = SHA256.Create();
byte[] saltedPassBytes = sha256.ComputeHash(rawSalted);
// Add the salt value to the salted hash.
byte[] dbPassword = new byte[saltedPassBytes.Length + saltBytes.Length];
saltedPassBytes.CopyTo(dbPassword, 0);
saltBytes.CopyTo(dbPassword, saltedPassBytes.Length);
return Convert.ToBase64String(dbPassword);
}
// BAD - Hash without a salt.
public static string HashPassword3(string password)
{
HashAlgorithm hashAlg = new SHA256CryptoServiceProvider();
byte[] passBytes = System.Text.Encoding.ASCII.GetBytes(password);
byte[] hashBytes = hashAlg.ComputeHash(passBytes);
return Convert.ToBase64String(hashBytes);
}
// GOOD - Hash with a salt.
public bool VerifyPasswordHash(string password, byte[] passwordHash, byte[] passwordSalt)
{
using(var hmac = new System.Security.Cryptography.HMACSHA512(passwordSalt))
{
var computedHash = hmac.ComputeHash(System.Text.Encoding.UTF8.GetBytes(password));
for(int i = 0;i<computedHash.Length;i++)
{
if (computedHash[i] != passwordHash[i])
return false;
}
return true;
}
}
public static byte[] GenerateSalt()
{
using (var rng = new RNGCryptoServiceProvider())
{
var randomNumber = new byte[SaltSize];
rng.GetBytes(randomNumber);
return randomNumber;
}
}
public static byte[] Combine(byte[] first, byte[] second)
{
// helper to combine two byte arrays
byte[] ret = new byte[first.Length + second.Length];
Buffer.BlockCopy(first, 0, ret, 0, first.Length);
Buffer.BlockCopy(second, 0, ret, first.Length, second.Length);
return ret;
}
// GOOD - Hash with a salt.
public static byte[] CalculateKeys(string password, string userid)
{
var utf16pass = System.Text.Encoding.UTF8.GetBytes(password);
var utf16sid = System.Text.Encoding.UTF8.GetBytes(userid);
var utf16sidfinal = new byte[utf16sid.Length + 2];
utf16sid.CopyTo(utf16sidfinal, 0);
utf16sidfinal[utf16sidfinal.Length - 2] = 0x00;
byte[] sha1bytes_password;
byte[] hmacbytes;
//Calculate SHA1 from user password
using (var sha1 = new SHA1Managed())
{
sha1bytes_password = sha1.ComputeHash(utf16pass);
}
var combined = Combine(sha1bytes_password, utf16sidfinal);
using (var hmac = new HMACSHA1(sha1bytes_password))
{
hmacbytes = hmac.ComputeHash(utf16sidfinal);
}
return hmacbytes;
}
private byte[] TryDecrypt(byte[] buffer, int offset, int length, byte[] password, int keyLen) {
byte[] key = new byte[16];
Array.Copy(SHA1.Create().ComputeHash(password, 0, password.Length), 0, key, 0, keyLen);
byte[] ret = Aes.Create().CreateDecryptor(key, null).TransformFinalBlock(buffer, offset, length);
return ret;
}
// GOOD - Use password hash without a salt having further processing.
public byte[] encrypt(byte[] pass, byte[] salt, byte[] blob) {
byte[] key = new byte[salt.Length + pass.Length];
Array.Copy(salt, 0, key, 0, salt.Length);
Array.Copy(pass, 0, key, salt.Length, pass.Length);
byte[] pkb = TryDecrypt(blob, 8, blob.Length - 8, key, 16);
return pkb;
}
public string CreatePasswordHash(string password, string saltkey)
{
var saltAndPassword = string.Concat(password, saltkey);
HashAlgorithm algorithm = SHA256.Create();
var hashByteArray = algorithm.ComputeHash(Encoding.UTF8.GetBytes(saltAndPassword));
return BitConverter.ToString(hashByteArray).Replace("-", "");
}
private string GetMD5HashBinHex (string toBeHashed)
{
MD5 hash = MD5.Create ();
byte[] result = hash.ComputeHash (Encoding.ASCII.GetBytes (toBeHashed));
StringBuilder sb = new StringBuilder ();
foreach (byte b in result)
sb.Append (b.ToString ("x2"));
return sb.ToString ();
}
// GOOD: Password concatenated with other information before hashing
public string CreatePasswordHash2(string username, string realm, string password)
{
string A1 = String.Format ("{0}:{1}:{2}", username, realm, password);
string HA1 = GetMD5HashBinHex (A1);
return HA1;
}
private byte[] Xor(byte[] array1, byte[] array2) {
var result = new byte[array1.Length];
for (int i = 0; i < array1.Length; i++) {
result[i] = (byte)(array1[i] ^ array2[i]);
}
return result;
}
// GOOD: Password hash without salt is further hashed with salt
public byte[] GetScrable(byte[] password, byte[] decodedSalt) {
var first20SaltBytes = new byte[20];
Array.Copy(decodedSalt, first20SaltBytes, 20);
var step1 = Sha1Utils.Hash(password);
var step2 = Sha1Utils.Hash(step1);
var step3 = Sha1Utils.Hash(first20SaltBytes, step2);
var scrambleBytes = Xor(step1, step3);
return scrambleBytes;
}
}

View File

@@ -0,0 +1,20 @@
edges
| HashWithoutSalt.cs:18:70:18:77 | access to parameter password : String | HashWithoutSalt.cs:20:49:20:56 | access to local variable passBuff |
| HashWithoutSalt.cs:38:28:38:72 | call to method GetBytes : Byte[] | HashWithoutSalt.cs:39:51:39:59 | access to local variable passBytes |
| HashWithoutSalt.cs:38:64:38:71 | access to parameter password : String | HashWithoutSalt.cs:38:28:38:72 | call to method GetBytes : Byte[] |
| HashWithoutSalt.cs:70:28:70:72 | call to method GetBytes : Byte[] | HashWithoutSalt.cs:71:48:71:56 | access to local variable passBytes |
| HashWithoutSalt.cs:70:64:70:71 | access to parameter password : String | HashWithoutSalt.cs:70:28:70:72 | call to method GetBytes : Byte[] |
nodes
| HashWithoutSalt.cs:18:70:18:77 | access to parameter password : String | semmle.label | access to parameter password : String |
| HashWithoutSalt.cs:20:49:20:56 | access to local variable passBuff | semmle.label | access to local variable passBuff |
| HashWithoutSalt.cs:38:28:38:72 | call to method GetBytes : Byte[] | semmle.label | call to method GetBytes : Byte[] |
| HashWithoutSalt.cs:38:64:38:71 | access to parameter password : String | semmle.label | access to parameter password : String |
| HashWithoutSalt.cs:39:51:39:59 | access to local variable passBytes | semmle.label | access to local variable passBytes |
| HashWithoutSalt.cs:70:28:70:72 | call to method GetBytes : Byte[] | semmle.label | call to method GetBytes : Byte[] |
| HashWithoutSalt.cs:70:64:70:71 | access to parameter password : String | semmle.label | access to parameter password : String |
| HashWithoutSalt.cs:71:48:71:56 | access to local variable passBytes | semmle.label | access to local variable passBytes |
subpaths
#select
| HashWithoutSalt.cs:20:49:20:56 | access to local variable passBuff | HashWithoutSalt.cs:18:70:18:77 | access to parameter password : String | HashWithoutSalt.cs:20:49:20:56 | access to local variable passBuff | $@ is hashed without a salt. | HashWithoutSalt.cs:18:70:18:77 | access to parameter password | The password |
| HashWithoutSalt.cs:39:51:39:59 | access to local variable passBytes | HashWithoutSalt.cs:38:64:38:71 | access to parameter password : String | HashWithoutSalt.cs:39:51:39:59 | access to local variable passBytes | $@ is hashed without a salt. | HashWithoutSalt.cs:38:64:38:71 | access to parameter password | The password |
| HashWithoutSalt.cs:71:48:71:56 | access to local variable passBytes | HashWithoutSalt.cs:70:64:70:71 | access to parameter password : String | HashWithoutSalt.cs:71:48:71:56 | access to local variable passBytes | $@ is hashed without a salt. | HashWithoutSalt.cs:70:64:70:71 | access to parameter password | The password |

View File

@@ -0,0 +1 @@
experimental/Security Features/CWE-759/HashWithoutSalt.ql

View File

@@ -0,0 +1,53 @@
using System;
using System.Security.Cryptography;
using System.Text;
internal static class Sha1Utils
{
public static byte[] Hash(string str)
{
var bytes = str == null ? new byte[0] : Encoding.UTF8.GetBytes(str);
return Hash(bytes);
}
public static byte[] Hash(byte[] bytes)
{
var sha1 = SHA1.Create();
var hashBytes = sha1.ComputeHash(bytes);
return hashBytes;
}
public static string HexStringFromBytes(byte[] bytes)
{
var sb = new StringBuilder();
foreach (var b in bytes)
{
var hex = b.ToString("x2");
sb.Append(hex);
}
return sb.ToString();
}
public static byte[] Hash(byte[] salt, byte[] str)
{
var salted = new byte[salt.Length + str.Length];
Array.Copy(salt, salted, salt.Length);
Array.Copy(str, 0, salted, salt.Length, str.Length);
return Hash(salted);
}
public static byte[] Xor(byte[] array1, byte[] array2)
{
var result = new byte[array1.Length];
for (int i = 0; i < array1.Length; i++)
{
result[i] = (byte)(array1[i] ^ array2[i]);
}
return result;
}
}

View File

@@ -0,0 +1,45 @@
namespace Windows.Security.Cryptography
{
public enum BinaryStringEncoding
{
Utf8,
Utf16LE,
Utf16BE
}
public static class CryptographicBuffer
{
public static Windows.Storage.Streams.IBuffer ConvertStringToBinary(string value, BinaryStringEncoding encoding) => throw null;
public static string EncodeToBase64String(Windows.Storage.Streams.IBuffer buffer) => throw null;
}
}
namespace Windows.Storage.Streams
{
public interface IBuffer {
public uint Capacity { get; }
public uint Length { get; set; }
}
}
namespace Windows.Security.Cryptography.Core
{
public sealed class CryptographicKey { }
public sealed class SymmetricKeyAlgorithmProvider
{
public CryptographicKey CreateSymmetricKey(Windows.Storage.Streams.IBuffer keyMaterial) => throw null;
}
public sealed class HashAlgorithmProvider {
public string AlgorithmName { get; }
public uint HashLength { get; }
public static HashAlgorithmProvider OpenAlgorithm(string algorithm) => throw null;
public Windows.Storage.Streams.IBuffer HashData(Windows.Storage.Streams.IBuffer data) => throw null;
}
}