Skip to content

Commit

Permalink
Merge branch 'OWASP:master' into master
Browse files Browse the repository at this point in the history
  • Loading branch information
sk3l10x1ng authored Sep 4, 2024
2 parents e82452a + 49f77b7 commit d3ceee6
Show file tree
Hide file tree
Showing 36 changed files with 840 additions and 194 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/spell-checker-pr.yml
Original file line number Diff line number Diff line change
Expand Up @@ -25,5 +25,5 @@ jobs:
with:
check_filenames: true
skip: "*.json,*.yml,*.apk,*.ipa,*.svg"
ignore_words_list: "aas,aaS,ba,bund,compliancy,firt,ist,keypair,ligh,Manuel,Manual,ro,ser,synopsys,theses,zuser,lief"
ignore_words_list: "aas,aaS,ba,bund,compliancy,firt,ist,keypair,ligh,Manuel,Manual,ro,ser,synopsys,theses,zuser,lief,EDE"
path: ${{ env.CHANGED_FILES }}
2 changes: 1 addition & 1 deletion .github/workflows/spell-checker.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,6 @@ jobs:
- uses: actions/checkout@v4
- uses: codespell-project/actions-codespell@master
with:
ignore_words_list: "aas,aaS,ba,bund,compliancy,firt,ist,keypair,ligh,Manuel,Manual,ro,ser,synopsys,theses,zuser,lief"
ignore_words_list: "aas,aaS,ba,bund,compliancy,firt,ist,keypair,ligh,Manuel,Manual,ro,ser,synopsys,theses,zuser,lief,EDE"
skip: "*.json,*.yml,*.apk,*.ipa,*.svg"
exclude_file: docs/contributing.md
29 changes: 29 additions & 0 deletions demos/android/MASVS-CRYPTO/MASTG-DEMO-0012/MASTG-DEMO-0012.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
---
platform: android
title: Weak Cryptographic Key Generation
code: [java]
id: MASTG-DEMO-0012
test: MASTG-TEST-0208
---

### Sample

{{ MastgTest.kt # MastgTest_reversed.java }}

### Steps

Let's run our @MASTG-TOOL-0110 rule against the sample code.

{{ ../../../../rules/mastg-android-weak-key-generation.yml }}

{{ run.sh }}

### Observation

The rule has identified some instances in the code file where cryptographic keys are being generated. The specified line numbers can be located in the reverse-engineered code for further investigation and remediation.

{{ output.txt }}

### Evaluation

The test fails because the key size of the RSA key is set to `1024` bits, and the size of the AES key is set to `128`, which is considered weak in both cases.
33 changes: 33 additions & 0 deletions demos/android/MASVS-CRYPTO/MASTG-DEMO-0012/MastgTest.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
package org.owasp.mastestapp

import android.util.Log
import android.content.Context
import android.security.keystore.KeyProperties
import android.util.Base64
import java.security.KeyPairGenerator
import java.security.SecureRandom
import javax.crypto.KeyGenerator
import javax.crypto.SecretKey

class MastgTest (private val context: Context){

fun mastgTest(): String {

val generator = KeyPairGenerator.getInstance(KeyProperties.KEY_ALGORITHM_RSA)
generator.initialize(1024, SecureRandom()) // for 1025 bit RSA Key
val keypair = generator.genKeyPair()
Log.d("Keypair generated RSA", Base64.encodeToString(keypair.public.encoded, Base64.DEFAULT))

val keyGen1 = KeyGenerator.getInstance("AES")
keyGen1.init(128) // for 128 bit AES key
val secretKey1: SecretKey = keyGen1.generateKey()

val keyGen2 = KeyGenerator.getInstance("AES")
keyGen2.init(256) // for 256 bit AES key
val secretKey2: SecretKey = keyGen2.generateKey()

return "Generated RSA Key:\n " + Base64.encodeToString(keypair.public.encoded, Base64.DEFAULT)+"Generated AES Key1\n "+ Base64.encodeToString(secretKey1.encoded, Base64.DEFAULT)+ "Generated AES Key2\n "+ Base64.encodeToString(secretKey2.encoded, Base64.DEFAULT);

}

}
41 changes: 41 additions & 0 deletions demos/android/MASVS-CRYPTO/MASTG-DEMO-0012/MastgTest_reversed.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
package org.owasp.mastestapp;

import android.content.Context;
import android.util.Base64;
import android.util.Log;
import java.security.KeyPair;
import java.security.KeyPairGenerator;
import java.security.SecureRandom;
import javax.crypto.KeyGenerator;
import javax.crypto.SecretKey;
import kotlin.Metadata;
import kotlin.jvm.internal.Intrinsics;

/* compiled from: MastgTest.kt */
@Metadata(d1 = {"\u0000\u0018\n\u0002\u0018\u0002\n\u0002\u0010\u0000\n\u0000\n\u0002\u0018\u0002\n\u0002\b\u0002\n\u0002\u0010\u000e\n\u0000\b\u0007\u0018\u00002\u00020\u0001B\r\u0012\u0006\u0010\u0002\u001a\u00020\u0003¢\u0006\u0002\u0010\u0004J\u0006\u0010\u0005\u001a\u00020\u0006R\u000e\u0010\u0002\u001a\u00020\u0003X\u0082\u0004¢\u0006\u0002\n\u0000¨\u0006\u0007"}, d2 = {"Lorg/owasp/mastestapp/MastgTest;", "", "context", "Landroid/content/Context;", "(Landroid/content/Context;)V", "mastgTest", "", "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 mastgTest() {
KeyPairGenerator generator = KeyPairGenerator.getInstance("RSA");
generator.initialize(1024, new SecureRandom());
KeyPair keypair = generator.genKeyPair();
Log.d("Keypair generated RSA", Base64.encodeToString(keypair.getPublic().getEncoded(), 0));
KeyGenerator keyGen1 = KeyGenerator.getInstance("AES");
keyGen1.init(128);
SecretKey secretKey1 = keyGen1.generateKey();
Intrinsics.checkNotNullExpressionValue(secretKey1, "generateKey(...)");
KeyGenerator keyGen2 = KeyGenerator.getInstance("AES");
keyGen2.init(256);
SecretKey secretKey2 = keyGen2.generateKey();
Intrinsics.checkNotNullExpressionValue(secretKey2, "generateKey(...)");
return "Generated RSA Key:\n " + Base64.encodeToString(keypair.getPublic().getEncoded(), 0) + "Generated AES Key1\n " + Base64.encodeToString(secretKey1.getEncoded(), 0) + "Generated AES Key2\n " + Base64.encodeToString(secretKey2.getEncoded(), 0);
}
}
15 changes: 15 additions & 0 deletions demos/android/MASVS-CRYPTO/MASTG-DEMO-0012/output.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@


┌─────────────────┐
│ 2 Code Findings │
└─────────────────┘

MastgTest_reversed.java
❯❱ weak_key_size
Cryptographic implementations with insufficient key length are being used.

27┆ KeyPairGenerator generator = KeyPairGenerator.getInstance("RSA");
28┆ generator.initialize(1024, new SecureRandom());
⋮┆----------------------------------------
31┆ KeyGenerator keyGen1 = KeyGenerator.getInstance("AES");
32┆ keyGen1.init(128);
1 change: 1 addition & 0 deletions demos/android/MASVS-CRYPTO/MASTG-DEMO-0012/run.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
NO_COLOR=true semgrep -c ../../../../rules/mastg-android-weak-key-generation.yml ./MastgTest_reversed.java --text -o output.txt
32 changes: 32 additions & 0 deletions demos/android/MASVS-CRYPTO/MASTG-DEMO-0015/MASTG-DEMO-0015.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
---
platform: android
title: Use of Hardcoded AES Key in SecretKeySpec with semgrep
tools: [semgrep]
code: [java]
---

### Sample

{{ MastgTest.kt }}

### Steps

Let's run our @MASTG-TOOL-0110 rule against the sample code.

{{ ../../../../rules/mastg-android-hardcoded-crypto-keys-usage.yaml }}

{{ run.sh }}

### Observation

The rule has identified one instance in the code file where hardcoded keys is used. The specified line numbers can be located in the reverse-engineered code for further investigation and remediation.

{{ output.txt }}

### Evaluation

The test fails because hardcoded cryptographic keys are present in the code. Specifically:

- On line 24, a byte array that represents a cryptographic key is directly hardcoded into the source code.
- This hardcoded key is then used on line 26 to create a `SecretKeySpec`.
- Additionally, on line 30, another instance of hardcoded data is used to create a separate `SecretKeySpec`.
28 changes: 28 additions & 0 deletions demos/android/MASVS-CRYPTO/MASTG-DEMO-0015/MastgTest.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
package org.owasp.mastestapp

import android.content.Context
import javax.crypto.Cipher
import javax.crypto.SecretKey
import javax.crypto.spec.SecretKeySpec
import android.util.Base64

class MastgTest(private val context: Context) {

fun mastgTest(): String {

// Bad: Use of a hardcoded key (from bytes) for encryption
val keyBytes = byteArrayOf(0x6C, 0x61, 0x6B, 0x64, 0x73, 0x6C, 0x6A, 0x6B, 0x61, 0x6C, 0x6B, 0x6A, 0x6C, 0x6B, 0x6C, 0x73) // Example key bytes
val cipher = Cipher.getInstance("AES/GCM/NoPadding")
val secretKey = SecretKeySpec(keyBytes, "AES")
cipher.init(Cipher.ENCRYPT_MODE, secretKey)

// Bad: Hardcoded key directly in code (security risk)
val badSecretKeySpec = SecretKeySpec("my secret here".toByteArray(), "AES")


// Returning results
return "SUCCESS!!\n\nThe keys were generated and used successfully with the following details:\n\n" +
"Hardcoded AES Encryption Key: ${Base64.encodeToString(keyBytes, Base64.DEFAULT)}\n" +
"Hardcoded Key from string: ${Base64.encodeToString(badSecretKeySpec.encoded, Base64.DEFAULT)}\n"
}
}
33 changes: 33 additions & 0 deletions demos/android/MASVS-CRYPTO/MASTG-DEMO-0015/MastgTest_reversed.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
package org.owasp.mastestapp;

import android.content.Context;
import android.util.Base64;
import javax.crypto.Cipher;
import javax.crypto.spec.SecretKeySpec;
import kotlin.Metadata;
import kotlin.jvm.internal.Intrinsics;
import kotlin.text.Charsets;

/* compiled from: MastgTest.kt */
@Metadata(d1 = {"\u0000\u0018\n\u0002\u0018\u0002\n\u0002\u0010\u0000\n\u0000\n\u0002\u0018\u0002\n\u0002\b\u0002\n\u0002\u0010\u000e\n\u0000\b\u0007\u0018\u00002\u00020\u0001B\r\u0012\u0006\u0010\u0002\u001a\u00020\u0003¢\u0006\u0002\u0010\u0004J\u0006\u0010\u0005\u001a\u00020\u0006R\u000e\u0010\u0002\u001a\u00020\u0003X\u0082\u0004¢\u0006\u0002\n\u0000¨\u0006\u0007"}, d2 = {"Lorg/owasp/mastestapp/MastgTest;", "", "context", "Landroid/content/Context;", "(Landroid/content/Context;)V", "mastgTest", "", "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 mastgTest() {
byte[] keyBytes = {108, 97, 107, 100, 115, 108, 106, 107, 97, 108, 107, 106, 108, 107, 108, 115};
Cipher cipher = Cipher.getInstance("AES/GCM/NoPadding");
SecretKeySpec secretKey = new SecretKeySpec(keyBytes, "AES");
cipher.init(1, secretKey);
byte[] bytes = "my secret here".getBytes(Charsets.UTF_8);
Intrinsics.checkNotNullExpressionValue(bytes, "this as java.lang.String).getBytes(charset)");
SecretKeySpec badSecretKeySpec = new SecretKeySpec(bytes, "AES");
return "SUCCESS!!\n\nThe keys were generated and used successfully with the following details:\n\nHardcoded AES Encryption Key: " + Base64.encodeToString(keyBytes, 0) + "\nHardcoded Key from string: " + Base64.encodeToString(badSecretKeySpec.getEncoded(), 0) + '\n';
}
}
18 changes: 18 additions & 0 deletions demos/android/MASVS-CRYPTO/MASTG-DEMO-0015/output.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@


┌─────────────────┐
│ 3 Code Findings │
└─────────────────┘

MastgTest_reversed.java
❯❯❱ hardcoded-crypto-key-test
Hardcoded cryptographic keys are found in use.

24┆ byte[] keyBytes = {108, 97, 107, 100, 115, 108, 106, 107, 97, 108, 107, 106, 108, 107, 108,
115};
25┆ Cipher cipher = Cipher.getInstance("AES/GCM/NoPadding");
26┆ SecretKeySpec secretKey = new SecretKeySpec(keyBytes, "AES");
⋮┆----------------------------------------
26┆ SecretKeySpec secretKey = new SecretKeySpec(keyBytes, "AES");
⋮┆----------------------------------------
30┆ SecretKeySpec badSecretKeySpec = new SecretKeySpec(bytes, "AES");
1 change: 1 addition & 0 deletions demos/android/MASVS-CRYPTO/MASTG-DEMO-0015/run.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
NO_COLOR=true semgrep -c ../../../../rules/mastg-android-hardcoded-crypto-keys-usage.yml ./MastgTest_reversed.java --text -o output.txt
38 changes: 38 additions & 0 deletions demos/ios/MASVS-CRYPTO/MASTG-DEMO-0011/MASTG-DEMO-0011.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
---
platform: ios
title: Uses of Weak Key Size in SecKeyCreateRandomKey with r2
code: [swift]
id: MASTG-DEMO-0011
test: MASTG-TEST-0209
---

### Sample

{{ MastgTest.swift }}

### Steps

When calling [`SecKeyCreateRandomKey`](https://developer.apple.com/documentation/security/1823694-seckeycreaterandomkey) the key size is specified in the [`kSecAttrKeySizeInBits`](https://developer.apple.com/documentation/security/ksecattrkeysizeinbits) attribute within the `parameters` dictionary. See [Key Generation Attributes](https://developer.apple.com/documentation/security/certificate_key_and_trust_services/keys/key_generation_attributes) for details.

1. Unzip the app package and locate the main binary file (@MASTG-TECH-0058), which in this case is `./Payload/MASTestApp.app/MASTestApp`.
2. Open the app binary with @MASTG-TOOL-0073 with the `-i` option to run this script.

{{ security_keysize.r2 }}

{{ run.sh }}

### Observation

The output contains the disassembled code of the function using `SecKeyCreateRandomKey`.

{{ output.txt }}

This function is pretty big so we just included the relevant part of the code that's right before the call to `SecKeyCreateRandomKey`. Note that we can see attributes being set in the `parameters` dictionary such as `kSecAttrKeySizeInBits` as `reloc.kSecAttrKeySizeInBits`. In radare2, this means that the symbol `kSecAttrKeySizeInBits` is not directly referenced by an absolute address but rather through a relocation entry. This entry will be resolved by the dynamic linker at runtime to the actual address where `kSecAttrKeySizeInBits` is located in memory.

### Evaluation

In the output we can see how the `kSecAttrKeySizeInBits` attribute is set to `1024` bits (0x400 in hexadecimal) using the `x8` register. This is later used to call `SecKeyCreateRandomKey`.

{{ evaluation.txt }}

The test fails because the key size is set to `1024` bits, which is considered weak for RSA encryption. The key size should be increased to `2048` bits or higher to provide adequate security against modern cryptographic attacks.
Binary file not shown.
90 changes: 90 additions & 0 deletions demos/ios/MASVS-CRYPTO/MASTG-DEMO-0011/MastgTest.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
import Foundation
import Security

struct MastgTest {
static func mastgTest(completion: @escaping (String) -> Void) {

// Step 1: Generate an RSA key pair with a 1024-bit key size
let tag = "org.owasp.mas.rsa-1014".data(using: .utf8)!
let keyAttributes: [String: Any] = [
kSecAttrKeyType as String: kSecAttrKeyTypeRSA,
kSecAttrKeySizeInBits as String: 1024, // Using 1024-bit RSA key
kSecPrivateKeyAttrs as String:
[kSecAttrIsPermanent as String: true, // to store it in the Keychain
kSecAttrApplicationTag as String: tag] // to find and retrieve it from the Keychain later
]

var error: Unmanaged<CFError>?
guard let privateKey = SecKeyCreateRandomKey(keyAttributes as CFDictionary, &error) else {
completion("Failed to generate private key: \(String(describing: error))")
return
}

guard let publicKey = SecKeyCopyPublicKey(privateKey) else {
completion("Failed to generate public key")
return
}

// Convert the private key to data (DER format)
guard let privateKeyData = SecKeyCopyExternalRepresentation(privateKey, &error) as Data? else {
completion("Failed to extract private key: \(String(describing: error))")
return
}

// Encode the private key for display
//let privateKeyBase64 = privateKeyData.base64EncodedString()
let privateKeyHex = privateKeyData.map { String(format: "%02hhx", $0) }.joined()

// Convert the public key to data (DER format)
guard let publicKeyData = SecKeyCopyExternalRepresentation(publicKey, &error) as Data? else {
completion("Failed to extract public key: \(String(describing: error))")
return
}

// Encode the public key for display
// let publicKeyBase64 = publicKeyData.base64EncodedString()
let publicKeyHex = publicKeyData.map { String(format: "%02hhx", $0) }.joined()

// Data to sign
let dataToSign = "This is a sample text".data(using: .utf8)!

// Step 2: Sign the data with the private key
guard let signature = SecKeyCreateSignature(
privateKey,
SecKeyAlgorithm.rsaSignatureMessagePKCS1v15SHA256,
dataToSign as CFData,
&error
) else {
completion("Signing failed: \(String(describing: error))")
return
}

// Convert signature to hex string for display
let signatureHex = (signature as Data).map { String(format: "%02hhx", $0) }.joined()

// Step 3: Verify the signature with the public key
let verificationStatus = SecKeyVerifySignature(
publicKey,
SecKeyAlgorithm.rsaSignatureMessagePKCS1v15SHA256,
dataToSign as CFData,
signature as CFData,
&error
)

let verificationResult = verificationStatus ? "Signature is valid." : "Signature is invalid."

let value = """
Original: \(String(data: dataToSign, encoding: .utf8)!)
Private Key (Hex): \(privateKeyHex)
Public Key (Hex): \(publicKeyHex)
Signature (Hex): \(signatureHex)
Verification: \(verificationResult)
"""

completion(value)
}
}
9 changes: 9 additions & 0 deletions demos/ios/MASVS-CRYPTO/MASTG-DEMO-0011/evaluation.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
│ │ 0x10000484c 080942f9 ldr x8, reloc.kSecAttrKeySizeInBits ; 0x10000c410 -> Load the address of kSecAttrKeySizeInBits into x8
│ │ 0x100004850 000140f9 ldr x0, [x8]
│ │ 0x100004854 e30b0094 bl fcn.1000077e0
│ │ 0x100004858 800605a9 stp x0, x1, [x20, 0x50]
│ │ 0x10000485c 48000090 adrp x8, reloc.Foundation.__DataStorage._bytes.allocator__UnsafeMutableRawPointer______ ; 0x10000c000
│ │ 0x100004860 089d41f9 ldr x8, reloc.Swift.Int ; 0x10000c338
│ │ 0x100004864 883e00f9 str x8, [x20, 0x78]
│ │ 0x100004868 08808052 mov w8, 0x400 -> Move 0x400 (1024 in decimal) into w8, the lower 32 bits of x8
│ │ 0x10000486c 883200f9 str x8, [x20, 0x60] -> Store the final value (1024-bit key size) into memory
Loading

0 comments on commit d3ceee6

Please sign in to comment.