-
-
Notifications
You must be signed in to change notification settings - Fork 2.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[MASWE-0020] Weak Encryption (by @appknox) #2910
base: master
Are you sure you want to change the base?
Changes from 11 commits
abb7d23
3ea5501
8d02243
3df4c9e
d0c5955
a6ef45b
54c1a9e
78777f8
057276f
8de7343
315fb5f
3416459
62d06ff
12bd55e
a47a60d
6580a35
5c541a3
64bf491
ebe5917
461a389
004ada0
346edb0
b5ab256
0554d0e
9447aec
371d8d1
e09958f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,33 @@ | ||||||
--- | ||||||
platform: android | ||||||
title: Weak Encryption | ||||||
id: MASTG-DEMO-0016 | ||||||
code: [java] | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
test: MASTG-TEST-0211 | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This can point to the existing test
Suggested change
|
||||||
--- | ||||||
|
||||||
### Sample | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add short sample description |
||||||
|
||||||
{{ MastgTest.kt # MastgTest_reversed.java }} | ||||||
|
||||||
### Steps | ||||||
|
||||||
Let's run our @MASTG-TOOL-0110 rule against the sample code. | ||||||
|
||||||
{{ ../../../../rules/mastg-android-weak-encryption.yaml }} | ||||||
|
||||||
{{ run.sh }} | ||||||
|
||||||
### Observation | ||||||
|
||||||
The rule has identified five instances in the code file where an insecure encryption is used. The specified line numbers can be located in the original code for further investigation and remediation. | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
|
||||||
{{ output.txt }} | ||||||
|
||||||
### Evaluation | ||||||
|
||||||
Review each of the reported instances. | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
|
||||||
- Line 37 seems the code utilize insecure DES algorithm. | ||||||
- Line 56 seems to utilize Cipher.getInstance("AES") defaults to ECB, which is insecure. | ||||||
- Line 79 seems the code utilize insecure 3DES algorithm. | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it's using it, right? It doesn't "seem" like it's using it. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,85 @@ | ||
package org.owasp.mastestapp | ||
|
||
import android.content.Context | ||
import java.security.Key | ||
import javax.crypto.Cipher | ||
import javax.crypto.SecretKeyFactory | ||
import javax.crypto.spec.DESKeySpec | ||
import javax.crypto.spec.DESedeKeySpec | ||
import javax.crypto.spec.SecretKeySpec | ||
import android.util.Base64 | ||
|
||
class MastgTest(private val context: Context) { | ||
|
||
// Vulnerable encryption using DES (weak algorithm) | ||
fun vulnerableDesEncryption(data: String): String { | ||
try { | ||
// Weak key for DES | ||
val keySpec = DESKeySpec("12345678".toByteArray()) | ||
val keyFactory = SecretKeyFactory.getInstance("DES") | ||
val secretKey: Key = keyFactory.generateSecret(keySpec) | ||
|
||
// Weak encryption algorithm (DES) and weak mode (ECB) | ||
val cipher = Cipher.getInstance("DES") | ||
cipher.init(Cipher.ENCRYPT_MODE, secretKey) | ||
|
||
val encryptedData = cipher.doFinal(data.toByteArray()) | ||
return Base64.encodeToString(encryptedData, Base64.DEFAULT) | ||
} catch (e: Exception) { | ||
return "Encryption error: ${e.message}" | ||
} | ||
} | ||
|
||
// Vulnerable AES with ECB mode | ||
fun vulnerableAesEcbEncryption(data: String): String { | ||
try { | ||
// Weak AES key (only for demonstration) | ||
val key = "1234567890123456".toByteArray() // 16 bytes key for AES | ||
|
||
// Using AES with ECB (default mode) | ||
val secretKeySpec = SecretKeySpec(key, "AES") | ||
val cipher = Cipher.getInstance("AES") | ||
cipher.init(Cipher.ENCRYPT_MODE, secretKeySpec) | ||
|
||
val encryptedData = cipher.doFinal(data.toByteArray()) | ||
return Base64.encodeToString(encryptedData, Base64.DEFAULT) | ||
} catch (e: Exception) { | ||
return "Encryption error: ${e.message}" | ||
} | ||
} | ||
|
||
// Vulnerable encryption using 3DES (Triple DES) | ||
fun vulnerable3DesEncryption(data: String): String { | ||
try { | ||
// Weak key for 3DES (24-byte key) | ||
val keySpec = DESedeKeySpec("123456789012345678901234".toByteArray()) // 24 bytes key | ||
val keyFactory = SecretKeyFactory.getInstance("DESede") | ||
val secretKey: Key = keyFactory.generateSecret(keySpec) | ||
|
||
// Weak encryption algorithm (3DES) | ||
val cipher = Cipher.getInstance("DESede") | ||
cipher.init(Cipher.ENCRYPT_MODE, secretKey) | ||
|
||
val encryptedData = cipher.doFinal(data.toByteArray()) | ||
return Base64.encodeToString(encryptedData, Base64.DEFAULT) | ||
} catch (e: Exception) { | ||
return "Encryption error: ${e.message}" | ||
} | ||
} | ||
|
||
fun mastgTest(): String { | ||
val sensitiveString = "Hello from the OWASP MASTG Test app." | ||
|
||
// Encrypt with weak DES | ||
val desEncryptedString = vulnerableDesEncryption(sensitiveString) | ||
|
||
// Encrypt with weak AES in ECB mode | ||
val aesEcbEncryptedString = vulnerableAesEcbEncryption(sensitiveString) | ||
|
||
// Encrypt with weak 3DES | ||
val tripleDesEncryptedString = vulnerable3DesEncryption(sensitiveString) | ||
|
||
// Returning the encrypted results | ||
return "DES Encrypted: $desEncryptedString\nAES ECB Encrypted: $aesEcbEncryptedString\n3DES Encrypted: $tripleDesEncryptedString" | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,98 @@ | ||
package org.owasp.mastestapp; | ||
|
||
import android.content.Context; | ||
import android.util.Base64; | ||
import java.security.Key; | ||
import javax.crypto.Cipher; | ||
import javax.crypto.SecretKeyFactory; | ||
import javax.crypto.spec.DESKeySpec; | ||
import javax.crypto.spec.DESedeKeySpec; | ||
import javax.crypto.spec.SecretKeySpec; | ||
import kotlin.Metadata; | ||
import kotlin.jvm.internal.Intrinsics; | ||
import kotlin.text.Charsets; | ||
|
||
/* compiled from: MastgTest.kt */ | ||
@Metadata(d1 = {"\u0000\u001a\n\u0002\u0018\u0002\n\u0002\u0010\u0000\n\u0000\n\u0002\u0018\u0002\n\u0002\b\u0002\n\u0002\u0010\u000e\n\u0002\b\u0005\b\u0007\u0018\u00002\u00020\u0001B\r\u0012\u0006\u0010\u0002\u001a\u00020\u0003¢\u0006\u0002\u0010\u0004J\u0006\u0010\u0005\u001a\u00020\u0006J\u000e\u0010\u0007\u001a\u00020\u00062\u0006\u0010\b\u001a\u00020\u0006J\u000e\u0010\t\u001a\u00020\u00062\u0006\u0010\b\u001a\u00020\u0006J\u000e\u0010\n\u001a\u00020\u00062\u0006\u0010\b\u001a\u00020\u0006R\u000e\u0010\u0002\u001a\u00020\u0003X\u0082\u0004¢\u0006\u0002\n\u0000¨\u0006\u000b"}, d2 = {"Lorg/owasp/mastestapp/MastgTest;", "", "context", "Landroid/content/Context;", "(Landroid/content/Context;)V", "mastgTest", "", "vulnerable3DesEncryption", "data", "vulnerableAesEcbEncryption", "vulnerableDesEncryption", "app_debug"}, k = 1, mv = {1, 9, 0}, xi = 48) | ||
/* loaded from: classes4.dex */ | ||
public final class MastgTest { | ||
public static final int $stable = 8; | ||
private final Context context; | ||
|
||
public MastgTest(Context context) { | ||
Intrinsics.checkNotNullParameter(context, "context"); | ||
this.context = context; | ||
} | ||
|
||
public final String vulnerableDesEncryption(String data) { | ||
Intrinsics.checkNotNullParameter(data, "data"); | ||
try { | ||
byte[] bytes = "12345678".getBytes(Charsets.UTF_8); | ||
Intrinsics.checkNotNullExpressionValue(bytes, "this as java.lang.String).getBytes(charset)"); | ||
DESKeySpec keySpec = new DESKeySpec(bytes); | ||
SecretKeyFactory keyFactory = SecretKeyFactory.getInstance("DES"); | ||
Key generateSecret = keyFactory.generateSecret(keySpec); | ||
Intrinsics.checkNotNullExpressionValue(generateSecret, "generateSecret(...)"); | ||
Key secretKey = generateSecret; | ||
Cipher cipher = Cipher.getInstance("DES"); | ||
cipher.init(1, secretKey); | ||
byte[] bytes2 = data.getBytes(Charsets.UTF_8); | ||
Intrinsics.checkNotNullExpressionValue(bytes2, "this as java.lang.String).getBytes(charset)"); | ||
byte[] encryptedData = cipher.doFinal(bytes2); | ||
String encodeToString = Base64.encodeToString(encryptedData, 0); | ||
Intrinsics.checkNotNullExpressionValue(encodeToString, "encodeToString(...)"); | ||
return encodeToString; | ||
} catch (Exception e) { | ||
return "Encryption error: " + e.getMessage(); | ||
} | ||
} | ||
|
||
public final String vulnerableAesEcbEncryption(String data) { | ||
Intrinsics.checkNotNullParameter(data, "data"); | ||
try { | ||
byte[] key = "1234567890123456".getBytes(Charsets.UTF_8); | ||
Intrinsics.checkNotNullExpressionValue(key, "this as java.lang.String).getBytes(charset)"); | ||
SecretKeySpec secretKeySpec = new SecretKeySpec(key, "AES"); | ||
Cipher cipher = Cipher.getInstance("AES"); | ||
cipher.init(1, secretKeySpec); | ||
byte[] bytes = data.getBytes(Charsets.UTF_8); | ||
Intrinsics.checkNotNullExpressionValue(bytes, "this as java.lang.String).getBytes(charset)"); | ||
byte[] encryptedData = cipher.doFinal(bytes); | ||
String encodeToString = Base64.encodeToString(encryptedData, 0); | ||
Intrinsics.checkNotNullExpressionValue(encodeToString, "encodeToString(...)"); | ||
return encodeToString; | ||
} catch (Exception e) { | ||
return "Encryption error: " + e.getMessage(); | ||
} | ||
} | ||
|
||
public final String vulnerable3DesEncryption(String data) { | ||
Intrinsics.checkNotNullParameter(data, "data"); | ||
try { | ||
byte[] bytes = "123456789012345678901234".getBytes(Charsets.UTF_8); | ||
Intrinsics.checkNotNullExpressionValue(bytes, "this as java.lang.String).getBytes(charset)"); | ||
DESedeKeySpec keySpec = new DESedeKeySpec(bytes); | ||
SecretKeyFactory keyFactory = SecretKeyFactory.getInstance("DESede"); | ||
Key generateSecret = keyFactory.generateSecret(keySpec); | ||
Intrinsics.checkNotNullExpressionValue(generateSecret, "generateSecret(...)"); | ||
Key secretKey = generateSecret; | ||
Cipher cipher = Cipher.getInstance("DESede"); | ||
cipher.init(1, secretKey); | ||
byte[] bytes2 = data.getBytes(Charsets.UTF_8); | ||
Intrinsics.checkNotNullExpressionValue(bytes2, "this as java.lang.String).getBytes(charset)"); | ||
byte[] encryptedData = cipher.doFinal(bytes2); | ||
String encodeToString = Base64.encodeToString(encryptedData, 0); | ||
Intrinsics.checkNotNullExpressionValue(encodeToString, "encodeToString(...)"); | ||
return encodeToString; | ||
} catch (Exception e) { | ||
return "Encryption error: " + e.getMessage(); | ||
} | ||
} | ||
|
||
public final String mastgTest() { | ||
String desEncryptedString = vulnerableDesEncryption("Hello from the OWASP MASTG Test app."); | ||
String aesEcbEncryptedString = vulnerableAesEcbEncryption("Hello from the OWASP MASTG Test app."); | ||
String tripleDesEncryptedString = vulnerable3DesEncryption("Hello from the OWASP MASTG Test app."); | ||
return "DES Encrypted: " + desEncryptedString + "\nAES ECB Encrypted: " + aesEcbEncryptedString + "\n3DES Encrypted: " + tripleDesEncryptedString; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
|
||
|
||
┌─────────────────┐ | ||
│ 3 Code Findings │ | ||
└─────────────────┘ | ||
|
||
MastgTest_reversed.java | ||
❯❱weak-encryption | ||
Weak encryption which defaults to ECB in use. | ||
|
||
37┆ Cipher cipher = Cipher.getInstance("DES"); | ||
⋮┆---------------------------------------- | ||
56┆ Cipher cipher = Cipher.getInstance("AES"); | ||
⋮┆---------------------------------------- | ||
79┆ Cipher cipher = Cipher.getInstance("DESede"); |
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. After all other changes were made. Rerun and push new outputs |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
NO_COLOR=true semgrep -c ../../../../rules/mastg-android-weak-encryption.yaml ./MastgTest_reversed.java --text -o output.txt |
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Rename to ...weak-encryption-algorithms.yaml |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,14 @@ | ||||||
rules: | ||||||
- id: weak-encryption | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
languages: | ||||||
- java | ||||||
severity: WARNING | ||||||
metadata: | ||||||
summary: This rule looks for weak encryption algorithm like DES, 3DES and default AES defaults to ECB. | ||||||
message: Weak encryption which defaults to ECB in use. | ||||||
pattern-either: | ||||||
- pattern: Cipher.getInstance("DES") | ||||||
- pattern: |- | ||||||
cpholguera marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
Cipher.getInstance("AES") | ||||||
- pattern: |- | ||||||
Cipher.getInstance("DESede") |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,23 @@ | ||
--- | ||
title: Weak Encryption | ||
sk3l10x1ng marked this conversation as resolved.
Show resolved
Hide resolved
|
||
platform: android | ||
id: MASTG-TEST-0211 | ||
type: [static] | ||
weakness: MASWE-0020 | ||
--- | ||
|
||
## Overview | ||
|
||
In this test case, we will look for the use of weak encryption in Android applications. To do this, we need to focus on the cryptographic implementations of algorithm such as DES, 3DES,Weak encryption modes (e.g. ECB) and Cipher.getInstance("AES") defaults to ECB. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This needs to be more elaborate, similar to https://mas.owasp.org/MASTG/tests-beta/android/MASVS-CRYPTO/MASTG-TEST-0208/ Be careful with missing whitespace and ensure code elements are enclosed in backticks, e.g., Also, try to use the |
||
|
||
## Steps | ||
|
||
1. Run a static analysis tool such as @MASTG-TOOL-0110 on the code and look for uses of the hardcoded cryptographic keys. | ||
|
||
## Observation | ||
|
||
The output should contain a list of locations where insecure encryption are used. | ||
|
||
## Evaluation | ||
|
||
The test case fails if you find any insecure encryption. |
Original file line number | Diff line number | Diff line change | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -25,3 +25,29 @@ | |||||||||||||||
|
||||||||||||||||
--- | ||||||||||||||||
|
||||||||||||||||
## Overview | ||||||||||||||||
cpholguera marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||
|
||||||||||||||||
Weak encryption refers to cryptographic systems or implementations that are vulnerable to attack, allowing unauthorised individuals to decrypt secured data. This weakness can be due to a number of reasons, including the use of outdated algorithms, deprecated encryption modes such as ECB and improper implementation practices such as the use of a non-random or empty Initialisation Vector (IV). | ||||||||||||||||
Check failure on line 30 in weaknesses/MASVS-CRYPTO/MASWE-0020.md GitHub Actions / markdown-lint-checkTrailing spaces
|
||||||||||||||||
|
||||||||||||||||
For example the use of TDEA (Triple Data Encryption Algorithm), which is often referred to as "Triple DES" or "3DES" is disallowed by NIST since end of 2023, due to known attacks like, "meet-in-the-middle", collision attacks and [Sweet32](https://nvd.nist.gov/vuln/detail/CVE-2016-2183). | ||||||||||||||||
|
||||||||||||||||
Similarly, hash functions such as MD5 and SHA-1, which were previously popular for ensuring data integrity, are now considered deprecated as attackers can generate hash collisions, potentially leading to data tampering. | ||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Remove |
||||||||||||||||
|
||||||||||||||||
## Impact | ||||||||||||||||
|
||||||||||||||||
- **Loss of Confidentiality**: Weak encryption may enable attackers to decipher and obtain sensitive information, resulting in unauthorized exposure and possible data breaches. | ||||||||||||||||
|
||||||||||||||||
- **Loss of Integrity**: Weak encryption can compromise the integrity of data, allowing adversaries to alter or manipulate the information without detection. | ||||||||||||||||
|
||||||||||||||||
## Mode of Introduction | ||||||||||||||||
|
||||||||||||||||
- **Use of Deprecated Algorithms** : Relying on outdated or weak cryptographic algorithms can allow threat actors to attack the cipher text, key or exploit known vulnerabilities in the algorithm, for example through brute force attacks. | ||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||
- **Insecure Modes of Operation**: Using modes that are considered deprecated increase the attack surface of encrypted information. For example the use of AES/ECB is deprecated as it divides the plaintext into blocks and encrypts each block separately using the same key. This makes the cipher text vulnerable to "known plaintext attacks" and leaks information about the structure of the original plaintext. | ||||||||||||||||
Check failure on line 45 in weaknesses/MASVS-CRYPTO/MASWE-0020.md GitHub Actions / markdown-lint-checkTrailing spaces
|
||||||||||||||||
- **Predictable Initialization Vectors (IVs)**: If IVs are not random or unique, they can be exploited in attacks like ciphertext injection or pattern recognition. This compromises the confidentiality of encrypted data, especially in modes like CBC (Cipher Block Chaining). | ||||||||||||||||
- **Weak Keys**: Short or easily guessable keys compromise encryption strength. The use of small key sizes (e.g., 56-bit keys in DES) can make the encryption susceptible to brute-force attacks. Best practices recommend keys of at least 256 bits for strong encryption. | ||||||||||||||||
- **Misuse of Non-Cryptographic Operations**: Relying on techniques such as XOR, Base64 encoding, or simple obfuscation methods for security purposes. These methods provide no actual encryption and can be easily reversed or decoded, exposing sensitive data. | ||||||||||||||||
|
||||||||||||||||
## Mitigations | ||||||||||||||||
|
||||||||||||||||
- Always use modern, well-established cryptographic libraries in mobile apps that follow best practices and offer cryptographic algorithms that are aligned with the recommendations by NIST. | ||||||||||||||||
- For example it is recommended to use secure modes of operations like GCM (Galois/Counter Mode) mode for AES with 256-bit keys, providing both encryption and authentication. | ||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can rework this section to mirror the previous one adding one mitigation for each of the modes of introduction.
Suggested change
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To go inline with MAST-DEMO-0018