Skip to content
New issue

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

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

Already on GitHub? # to your account

Fix heap buffer overflow in selReadStream (detected by clang address sanitizer) #499

Merged
merged 1 commit into from
May 6, 2020

Conversation

stweil
Copy link
Collaborator

@stweil stweil commented May 3, 2020

No description provided.

@stweil
Copy link
Collaborator Author

stweil commented May 3, 2020

The fix for tiffio breaks pixcomp_reg, therefore this needs separate examination. I'll remove that commit from the pull request.

@stweil stweil changed the title Fix two runtime errors (detected by clang sanitizers) Fix heap buffer overflow in selReadStream (detected by clang sanitizers) May 3, 2020
src/sel1.c Show resolved Hide resolved
@@ -1431,17 +1431,14 @@ SEL *sel;

if (fgets(linebuf, sizeof(linebuf), fp) == NULL)
return (SEL *)ERROR_PTR("error reading into linebuf", procName, NULL);
selname = stringNew(linebuf);
sscanf(linebuf, " ------ %200s ------", selname);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

... and here 201 bytes were written to selname.

@stweil stweil changed the title Fix heap buffer overflow in selReadStream (detected by clang sanitizers) Fix heap buffer overflow in selReadStream (detected by clang address sanitizer) May 3, 2020
@DanBloomberg
Copy link
Owner

I found and fixed the int32 --> uint32 issue before reading your PR.

Thank you for this, and for fixing the selname issue.

Seems I can't merge because of a build issue

@stweil
Copy link
Collaborator Author

stweil commented May 4, 2020

The build issue on Ubuntu looks unrelated to this pull request. @egorpugin, can you help, please?

@egorpugin
Copy link
Collaborator

Ignore it. Error is unrelated to leptonica. It's on sw side, I'll check.

@egorpugin
Copy link
Collaborator

Should be ok now.

selio_reg triggers a heap buffer overflow when sscanf tries to write 201 bytes into a 24 byte string.
It can be detected when the code is compiled with the address sanitizer:

    ==19856==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x603000001288 at pc 0x00000044462b bp 0x7fffffffddf0 sp 0x7fffffffd5a0
    WRITE of size 201 at 0x603000001288 thread T0
    0x603000001288 is located 0 bytes to the right of 24-byte region [0x603000001270,0x603000001288)

Signed-off-by: Stefan Weil <sw@weilnetz.de>
@DanBloomberg DanBloomberg merged commit 076971f into DanBloomberg:master May 6, 2020
@DanBloomberg
Copy link
Owner

I just realized this was waiting.
Thank you.

@stweil stweil deleted the fuzzing branch December 4, 2020 13:30
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants