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

Buffer overflow and uninitialised memory use in infer_mime_type_from_contents if xdg-mime doesn't print anything #243

Open
Noisytoot opened this issue Nov 18, 2024 · 2 comments

Comments

@Noisytoot
Copy link

    /* 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).

@Noisytoot
Copy link
Author

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

@bugaevc
Copy link
Owner

bugaevc commented Nov 18, 2024

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.

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

No branches or pull requests

2 participants