bugaevc / wl-clipboard

Command-line copy/paste utilities for Wayland
GNU General Public License v3.0
1.61k stars 60 forks source link

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

Open Noisytoot opened 3 hours ago

Noisytoot commented 3 hours ago
    /* 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 commented 2 hours ago

Here's a fix:

From 23506dae5255b84e134d70aaed707dbd2db652eb Mon Sep 17 00:00:00 2001
From: Ron Nazarov <ron@noisytoot.org>
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 commented 2 hours ago

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.