Buffer overflow and uninitialised memory use in infer_mime_type_from_contents if xdg-mime doesn't print anything
/* Read the result */
char *res = malloc(256);
size_t len = read(pipefd[0], res, 256);
/* Trim the newline */
len--;
res[len] = 0;
close(pipefd[0]);
if (str_has_prefix(res, "inode/")) {
free(res);
return NULL;
}
return res;
If xdg-mime doesn't print anything (len == 0), len-- will cause
len to underflow and the next line will try to set
res[18446744073709551615] to 0. res will then be returned
pointing to uninitialised memory, resulting in either another buffer
overflow or the mime type being filled with junk (which will prevent
it from being pasted by most programs).
Here's a fix:
From 23506dae5255b84e134d70aaed707dbd2db652eb Mon Sep 17 00:00:00 2001
From: Ron Nazarov <[email protected]>
Date: Mon, 18 Nov 2024 14:22:54 +0000
Subject: [PATCH] Fix buffer overflow in infer_mime_type_from_contents
Return early if len == 0 and don't attempt to trim a nonexistent
newline.
Fixes https://github.com/bugaevc/wl-clipboard/issues/243
---
src/util/files.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/src/util/files.c b/src/util/files.c
index d2f1c4d..574af49 100644
--- a/src/util/files.c
+++ b/src/util/files.c
@@ -187,6 +187,10 @@ char *infer_mime_type_from_contents(const char *file_path) {
/* Read the result */
char *res = malloc(256);
size_t len = read(pipefd[0], res, 256);
+ if (len == 0) {
+ free(res);
+ return NULL;
+ }
/* Trim the newline */
len--;
res[len] = 0;
--
2.46.0
Indeed, I suppose this could be made more robust, thanks. We should also be reading in a loop rather than once, and checking that the byte we're trimming is indeed a newline.
Pushed as https://github.com/bugaevc/wl-clipboard/commit/e8082035dafe0241739d7f7d16f7ecfd2ce06172 with a couple of tweaks, thanks!