Harden QR decoder against a crafted-image hang (and a NULL memcpy) - #159
Open
AlexandrKhromov2005 wants to merge 3 commits into
Open
Harden QR decoder against a crafted-image hang (and a NULL memcpy)#159AlexandrKhromov2005 wants to merge 3 commits into
AlexandrKhromov2005 wants to merge 3 commits into
Conversation
On the first quirc_resize() the previous image buffer q->image is NULL and q->w == q->h == 0, so min == 0 and the copy becomes memcpy(image, NULL, 0). That is undefined behaviour (memcpy's source is declared nonnull) and is flagged by UBSan on the first decode of every fresh quirc handle. Skip the copy when there is nothing to copy.
find_alignment_pattern() spirals outward while step_size*step_size < size_estimate*100. size_estimate is derived from coordinates produced by the perspective transform; a degenerate transform (near-zero denominator) makes those coordinates huge, so size_estimate becomes enormous and the loop runs for hundreds of millions of iterations. A 246x246 image (about the size of a real screenshot QR) makes quirc_end() run for over 90 seconds on ~28 MB of RAM - a pure CPU hang on attacker-supplied image data. The alignment pattern must lie within the image, so cap the search radius at the image size. Verified with a fuzzer-found reproducer: the 90s+ hang drops to ~12 ms and real QR codes still decode. Found by fuzzing (libFuzzer + ASan/UBSan).
kaworu
approved these changes
Aug 19, 2026
kaworu
left a comment
kaworu
left a comment
Collaborator
There was a problem hiding this comment.
Thanks for the PR @AlexandrKhromov2005, couple of nitpicking comments, but the patch LGTM.
| * radius at the image size. | ||
| */ | ||
| while (step_size * step_size < size_estimate * 100) { | ||
| int max_step = (q->w > q->h ? q->w : q->h) + 1; |
Collaborator
There was a problem hiding this comment.
const?
Suggested change
| int max_step = (q->w > q->h ? q->w : q->h) + 1; | |
| const int max_step = (q->w > q->h ? q->w : q->h) + 1; |
| * new buffer when the new size is smaller, hence the min computation. | ||
| */ | ||
| (void)memcpy(image, q->image, min); | ||
| if (min) |
Collaborator
There was a problem hiding this comment.
nitpicking
Suggested change
| if (min) | |
| if (min > 0) |
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Harden the QR decoder against a crafted-image hang (and a NULL memcpy)
Two fixes found by fuzzing quirc with libFuzzer + ASan/UBSan.
1. DoS: unbounded alignment-pattern spiral (
identify.c).find_alignment_pattern()spirals outward whilestep_size*step_size < size_estimate*100.size_estimateis derived from coordinates produced by the perspective transform; a degeneratetransform (near-zero denominator) makes those coordinates huge, so
size_estimatebecomes enormousand the loop runs for hundreds of millions of iterations. A 246×246 image (about the size of a
real screenshot QR) makes
quirc_end()run for over 90 seconds on ~28 MB of RAM — a pure CPUhang on attacker-supplied image data. Since the alignment pattern must lie within the image, this
caps the search radius at the image size. Verified with the fuzzer reproducer: the 90s+ hang drops
to ~12 ms and real QR codes still decode.
2. UB:
memcpy(dst, NULL, 0)on first resize (quirc.c).On the first
quirc_resize(),q->imageis NULL andq->w == q->h == 0, somin == 0and thecopy is
memcpy(image, NULL, 0)— UB (memcpy's source is declarednonnull), flagged by UBSan onthe first decode of every fresh handle. Guarded with
if (min).These reproduce on current
master. Both are also present in copies of quirc vendored by downstreamprojects (I hit #1 fuzzing one such client). Minimal reproducer for the hang available on request.