Disallow non-hex characters in fromHex

[wjaaaaaaat]

It appears that fromHex treats non-hex characters as delimiters, meaning invalid hex is happily and silently parsed. As hex is often the default option, frustrations may result: #1918

My edits simply change the "Auto" option from parsing any non-hex character as a delimiter to recognizing either whitespace or "0x" as delimiters.

I am a little worried this might accidentally break something that relies on this quirky functionality, but that seems unlikely.

[wjaaaaaaat]

Perhaps an option should be included that produces the original functionality (of recognizing any non-hex character as a delimiter) in case anyone was using it, but I have not implemented that.

[wjaaaaaaat]

A check is failing because of these lines:

CyberChef/tests/node/tests/operations.mjs

Line 619 in fed10f3

assert.strictEqual(chef.HMAC("On Cloud Nine", {key: "idea"}).toString(), "e15c268b4ee755c9e52db094ed50add7");

CyberChef/tests/node/tests/operations.mjs

Line 843 in fed10f3

chef.scrypt("Playing For Keeps", {salt: {string: "salty", option: "Hex"}}).toString(),

The inputs of "idea" and "salty" are parsed as hex equivalently to "dea" and "a". Is this intentional, meant to test this quirk of the hex parsing functionality?

[a3957273]

Hey @wjaaaaaaat, thanks for the contribution. This is... definitely a challenge. CyberChef users often have to deal with incomplete / invalid inputs and sometimes "best attempt" is better than an absolute rejection. On the other hand, I also want to know I'm making that trade-off, and hiding errors is not a good solution.

Other operators have offered this as clear options, which I think would also work here. Operators like From Base64 have a strict mode checkbox that throws an error on invalid text. It also has a checkbox to remove non-conforming text from the output. Would you be able to change this PR to offer that functionality? Would also resolve the failing test.

[wjaaaaaaat]

Hi! Thanks for your quick reply.

It seems that to add checkboxes like in the "From Base64" operator, we would need to add arguments to every single operator with an input that may be interpreted from hex. In other words, "From Base64" was more easily able to include this option because it is a feature of the operator, not the utility function that is ultimately used.

As I have little experience with CyberChef's code, this edit seems risky/unsustainable. However, I have a possible alternative in mind. Are there any functionalities to allow the recipe to go through but give a warning to the user? If not, it almost seems more worth it to implement this than to add checkboxes in the same way as in "From Base64".

If you have something else in mind, let me know!

[wjaaaaaaat]

Another point: many, many operations (such as AES Encrypt) use inputs that may be formatted as hex, but do not allow the user to change how the hex is parsed. I think it does not make much sense to allow any non-hex character to act as a delimiter in this case.

On the other hand, I also want to know I'm making that trade-off, and hiding errors is not a good solution.

Actually, I changed my mind after a bit of thinking. I now believe the best solution is to add an "ignore non-alphabet characters" checkbox to the "From Hex" operator only.

[wjaaaaaaat]

The failing checks are still not resolved, but to be honest, I feel that they should be changed.

CyberChef users often have to deal with incomplete / invalid inputs and sometimes "best attempt" is better than an absolute rejection.

How often would one need CyberChef to strip out the non-hex characters for them to use in operations like "HMAC"?

[ladderlogix]

The checks are only failing as the input mode selected is set wrong or not at all. HMAC lets it be default (hex) instead of specifying it to be UTF8 and Scythe has the incorrect one selected and should be changed to UTF8.

I agree this is a nice addition to cyberchef and would like to see this pull request added