Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix stack corruption and bus errors while scanning oversized QR codes #87

Merged
merged 1 commit into from
Sep 20, 2020

Conversation

claudiofelber
Copy link
Contributor

Quirc sometimes falsely detects very large QR codes (false positives). This happens for example when the camera of a smartphone is facing downwards and is recording an almost black image. When the library is trying to decode such a large QR code it accesses more memory than has previously been allocated (quirc_code→cell_bitmap). This leads to stack corruption and bus errors.

This pull requests addresses the issue by first defining a maximum supported QR code version number and defining QUIRC_MAX_BITMAP based on this maximum version number instead of using a fixed number of bytes. Later in measure_timing_pattern() QR codes exceeding the maximum allowed version number are ignored and the decoding process is aborted. This also shortens the time needed to identify and reject a bad QR code.

Copy link
Collaborator

@kaworu kaworu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks for the PR.

Side question: would you have such an image (false positive) available? I would be interested for testing purposes.

@claudiofelber
Copy link
Contributor Author

The attached image is an example of a false positive. Quirc believes to detect two QR codes: one with version 49 and one with version 46. I have also attached an image of the binarized image.

I think with the binarization method used in earlier versions of the library, this image example would not have lead to a problem because it would have been entirely black after the binarization, although the potential to crash would if course have been there. The new Otsu-based thresholding algorithm I contributed last year heavily improved binarization of low-light and unevenly lighted images but at the same time also increased the risk for the error, this pull request addresses, to happen.

grid_size_too_large
binarized

@claudiofelber claudiofelber mentioned this pull request Sep 16, 2020
@dlbeer dlbeer merged commit 92026d2 into dlbeer:master Sep 20, 2020
@dlbeer
Copy link
Owner

dlbeer commented Sep 20, 2020

Thanks for this. This was a pretty basic error -- I'm surprised nobody noticed it earlier!

kaworu added a commit to kaworu/node-quirc that referenced this pull request Sep 21, 2020
@kaworu
Copy link
Collaborator

kaworu commented Sep 21, 2020

And thanks for the test case, I've added it as a regression test to node-quirc: https://github.com/kAworu/node-quirc/blob/master/test/index.js#L314-L335

@claudiofelber
Copy link
Contributor Author

You're both very welcome. Thank you for providing such a great library!

kaworu added a commit to kaworu/node-quirc that referenced this pull request Sep 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants