fix: stop Parse QR Code from participating in Magic (#2610)#2613
Conversation
Parse QR Code declared a `checks` regex that matched any JPEG, PNG, GIF, WEBP or BMP magic bytes. Magic aggregates every operation with a `checks` property and runs them, so any image input triggered a full QR parse attempt that produced a 'Could not read a QR code from the image' warning in the browser console for every image, even when no QR code was present. There is no cheap way to detect a QR code without attempting a full parse, so Parse QR Code should not participate in Magic. Users can add it manually when they know the image contains a QR code. Adds a regression test asserting Parse QR Code declares no `checks`.
|
@GCHQDeveloper581 @tomdev123 @np748293 would appreciate your views here. This operation has been part of magic for quite a long time (since #515 ). It doesn't seem to be an accident - it was specifically added, so we should be making a conscious decision if we are to remove it. |
|
I originally suggested removing it from Magic when I raised Issue #2610, and have given my rationale there (and the mitigation). However, as more instances of Magic detection running amok are found - (#2410, #2642) I'm beginning to think that the correct response is to alter Magic to catch OperationError without reporting the error, and simply to remove the operation from consideration for that stage of the processing. |
|
Thinking this through again, I stand by my assertions in the "Additional Context" section of #2610. There is no simple way of detecting whether an image contains a QR code, and therefore carrying out a computationally expensive Operation on the off chance that it may contain one is not sensible when the Mark 1 eyeball applied to the image will immediately recognise the image as a QR code, allowing the operation to be added manually. |
GCHQDeveloper581
left a comment
GCHQDeveloper581
left a comment
There was a problem hiding this comment.
Looks good!
Thanks for your contribution.
3 participants
Description
Parse QR Codedeclared achecksregex that matched any JPEG, PNG, GIF, WEBP or BMP magic bytes.Magic._generateOpCriteria(insrc/core/lib/Magic.mjs) aggregates every operation that exposes achecksproperty and runs each against the input, so any image fed to Magic (which happens implicitly wheneverMagicis in the recipe, including via the URL/QueryString demo) triggered a full QR parse. The QR decoder then logged[BGChefWorker]: Could not read a QR code from the image.to the browser console for every image that didn't contain a QR code.This PR removes the
this.checksarray fromParseQRCode's constructor (leaving a comment that points at the issue). There is no cheap way to know an image contains a QR code without actually attempting to decode one, so it can't be a sensible Magic candidate. The operation itself is unchanged and every other operation is unaffected; only Magic's automatic dispatch stops attempting QR parsing. Users who know their image has a QR code can still addParse QR Codeto the recipe manually.Behaviour before: loading the 1px-PNG demo URL from the issue with DevTools open prints
[BGChefWorker]: Could not read a QR code from the image.Behaviour after: same URL, console is silent; Magic still recognises the input as an image and lists the usual image-render suggestions.
Existing Issue
Fixes #2610.
Screenshots
No visual changes to CyberChef. The only observable difference is the absence of a console error message; no UI is affected.
AI disclosure
This change was written with the assistance of Claude. I have reviewed the diff and the test, understand the root cause and the fix, and take responsibility for the submitted code.
Test Coverage
Adds
tests/node/tests/ParseQRCode.mjswith an API-level regression test asserting thatOperationConfig["Parse QR Code"]declares nochecks. The test fails onmasterwith the originalchecksregex and passes with this change (verified both directions). The full node and operations suites still pass locally (255 and 2057 tests respectively).