gchq / CyberChef

The Cyber Swiss Army Knife - a web app for encryption, encoding, compression and data analysis
https://gchq.github.io/CyberChef
Apache License 2.0
28.93k stars 3.25k forks source link

Wrong AES Encrypt output on inputs with special characters #1735

Open ncoder-1 opened 8 months ago

ncoder-1 commented 8 months ago

Describe the bug When using the "AES Encrypt" (input parameter set to Raw) operation with a UTF-8 input text which has special characters (in this case an é), the output is different from 3 different sources (openssl, botan and the cryptii online encrypter), all of which produce the same output.

If you perform a "To Hex" operation before the "AES Encrypt" operation, and set the input paramenter to Hex, it then produces the expected output.

To Reproduce

  1. Go to CyberChef
  2. Add an "AES Encrypt" operation
  3. Set key to B4294F848BD875E697FAF7CFAF40D102C733066C11C4356029710C9D5EBF26E7
  4. Set IV to 00000000000000000000000000000000
  5. In the input, set input character encoding to UTF-8
  6. Add emporté in as the input text
  7. The output will then be d2c844670dcf3ed9fe28f764e631fcb4 which is wrong.
  8. Add a "To Hex" operation before the "AES Encrypt"
  9. Set the "AES Encrypt" input to To Hex
  10. Output will be 1dfbb577dca5a8f207cf9fd4b0da7aee which is right.

Expected behaviour I would expect that it accepts special characters within UTF-8 when it is selected as the input encoding, such as below:

With botan cli:

echo "emporté" > input.txt
truncate -s -1 input.txt
botan cipher --cipher=AES-256/CBC --key=B4294F848BD875E697FAF7CFAF40D102C733066C11C4356029710C9D5EBF26E7 --nonce=00000000000000000000000000000000 input.txt > out.enc
xxd -ps out.enc

Produces: 1dfbb577dca5a8f207cf9fd4b0da7aee

With openssl enc:

echo "emporté" > input.txt
truncate -s -1 input.txt
openssl enc -aes-256-cbc -nosalt -in input.txt -out out.enc -p -K "B4294F848BD875E697FAF7CFAF40D102C733066C11C4356029710C9D5EBF26E7" -iv "00000000000000000000000000000000"
xxd -ps out.enc

Produces: 1dfbb577dca5a8f207cf9fd4b0da7aee

And obviously the same output with same parameters on cryptii

Screenshots Wrong: Screenshot from 24 02 24 15:27:31

Right: Screenshot from 24 02 24 15:27:44

Desktop (if relevant, please complete the following information):

zb3 commented 6 months ago

Since the inputType of the AESEncrypt operation is set to string (unlike ToHex which has inputType set to ArrayBuffer), there's no conversion performed using input-dependent encoding, hence it appears to be the lack of a feature, not a bug... You'd need to UTF8 encode the input first, but that... also does nothing since inputType equal to string makes CyberChef decode it automatically..

I believe the inputType should be ArrayBuffer (after all, the cipher operates on binary data), but it seems that the string value is used basically everywhere.. sad to see this project not-really-maintained.. ;/

diff --git a/src/core/operations/AESEncrypt.mjs b/src/core/operations/AESEncrypt.mjs
index 7b52ff0..101a2e7 100644
--- a/src/core/operations/AESEncrypt.mjs
+++ b/src/core/operations/AESEncrypt.mjs
@@ -6,6 +6,7 @@

 import Operation from "../Operation.mjs";
 import Utils from "../Utils.mjs";
+import { fromHex } from "../lib/Hex.mjs";
 import forge from "node-forge";
 import OperationError from "../errors/OperationError.mjs";

@@ -24,7 +25,7 @@ class AESEncrypt extends Operation {
         this.module = "Ciphers";
         this.description = "Advanced Encryption Standard (AES) is a U.S. Federal Information Processing Standard (FIPS). It was selected after a 5-year process where 15 competing designs were evaluated.<br><br><b>Key:</b> The following algorithms will be used based on the size of the key:<ul><li>16 bytes = AES-128</li><li>24 bytes = AES-192</li><li>32 bytes = AES-256</li></ul>You can generate a password-based key using one of the KDF operations.<br><br><b>IV:</b> The Initialization Vector should be 16 bytes long. If not entered, it will default to 16 null bytes.<br><br><b>Padding:</b> In CBC and ECB mode, PKCS#7 padding will be used.";
         this.infoURL = "https://wikipedia.org/wiki/Advanced_Encryption_Standard";
-        this.inputType = "string";
+        this.inputType = "ArrayBuffer";
         this.outputType = "string";
         this.args = [
             {
@@ -89,7 +90,7 @@ class AESEncrypt extends Operation {
     }

     /**
-     * @param {string} input
+     * @param {ArrayBuffer} input
      * @param {Object[]} args
      * @returns {string}
      *
@@ -112,7 +113,10 @@ The following algorithms will be used based on the size of the key:
   32 bytes = AES-256`);
         }

-        input = Utils.convertToByteString(input, inputType);
+        if (inputType === 'Hex') {
+            input = Utils.byteArrayToChars(
+                fromHex(Utils.arrayBufferToStr(input)));
+        }

         const cipher = forge.cipher.createCipher("AES-" + mode, key);
         cipher.start({
ncoder-1 commented 6 months ago

Good catch, and 100% agree with your last statement...

a3957273 commented 6 months ago

Hey @zb3 this repository is becoming more maintained, it's just a lot of work to go through multiple years of backlog. If you would be willing to make your change into a pull request I'd happily test / approve it!

a3957273 commented 6 months ago

Also, we're accepting other individuals into the 'triager' role to hopefully share some of the load. Would you be willing to help review / test existing pull requests? Let me know if you want to get added to the role.

SaeeBiwalkar commented 1 month ago

Is this bug still open? I want to work on this bug.