-
Notifications
You must be signed in to change notification settings - Fork 90
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
Invalid free wild pointer lead to DOS in load_png in loader.c #134
Comments
CVE-2020-11721 was assigned for this issue. |
I'm not sure of the validity of this bug, @carnil, @sleicasper. The maintainer of this project has been absent for over a year and now I'm de facto maintainer. (See #154.) I've patched the other CVE, see libsixel#8. This one doesn't seem valid to me though. I need more information. I don't get the crash. I get a I get:
If you can still repro the bug, is this a sufficient fix? diff --git a/src/loader.c b/src/loader.c
index abc27f8..ea93daa 100644
--- a/src/loader.c
+++ b/src/loader.c
@@ -630,7 +630,9 @@ load_png(unsigned char /* out */ **result,
cleanup:
png_destroy_read_struct(&png_ptr, &info_ptr,(png_infopp)0);
- sixel_allocator_free(allocator, rows);
+ if (rows != NULL) {
+ sixel_allocator_free(allocator, rows);
+ }
return status;
} If not, please give more info. If it's fixed, no big deal, I'll request CVE mark the bug invalid… |
@ctrlcctrlv This bug is reproducible at the time of reporting, which is v1.8.6. |
Please give a full command line. I'm attempting reproduction at 1.8.6 plus a bunch of PR's that should be irrelevant, call it libsixel@50206ad. |
Also, what is your libpng version? Mine is 1.6.37 on Arch Linux. My working theory right now is that this is not our bug, it's a libpng bug that they fixed recently which had the side effect of causing this bug. However, please feel free to explain to me how to reproduce so I can solve it if I'm wrong and close this out. |
Wait. Is what you're saying that if That doesn't seem to be the executed code path, but I can fix it. Is this what you mean @sleicasper ? |
@ctrlcctrlv yes, that is what I mean. 'rows' can be a wild pointer. After executing 'goto cleanup', it can be freed. |
Except that's not the executed codepath for your PoC? I don't mind patching it, but I don't understand how you got that from your PoC. |
See? I get a libpng error.
|
Anyway @sleicasper, I've merged libsixel@e71aacc which I consider as closing this. It closes libsixel#9 in the fork. I've released version 1.9.0. |
It seems to be a However, it looks like The patch looks good to me, it should prevent |
load_png
has a pointerrows
, which should be set to NULL, otherwisecleanup
code would use(callingfree
) it.binary: img2sixel
file: loader.c
function: load_png
poc:
poc.zip
result:
The text was updated successfully, but these errors were encountered: