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 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
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.

None yet

3 participants