From d8782cc5c22d83f4a4305f40d85a84f47c89f314 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 10 Aug 2021 14:37:00 +0200 Subject: [PATCH 1/2] memory-util: add mempmem_safe() This is like memmem_safe() but returns a pointer after the needle, instead to the beginning of the needle. This is then used at one place. Not much, but it makes me sleep safer at night, as it avoids the manual counting done so far. --- src/basic/memory-util.h | 10 ++++++++++ src/boot/bootctl.c | 3 +-- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/src/basic/memory-util.h b/src/basic/memory-util.h index 0b04278ab4..e2b543cb8f 100644 --- a/src/basic/memory-util.h +++ b/src/basic/memory-util.h @@ -71,6 +71,16 @@ static inline void *memmem_safe(const void *haystack, size_t haystacklen, const return memmem(haystack, haystacklen, needle, needlelen); } +static inline void *mempmem_safe(const void *haystack, size_t haystacklen, const void *needle, size_t needlelen) { + const uint8_t *p; + + p = memmem_safe(haystack, haystacklen, needle, needlelen); + if (!p) + return NULL; + + return (uint8_t*) p + needlelen; +} + #if HAVE_EXPLICIT_BZERO static inline void* explicit_bzero_safe(void *p, size_t l) { if (l > 0) diff --git a/src/boot/bootctl.c b/src/boot/bootctl.c index 264189c36b..9283e2013c 100644 --- a/src/boot/bootctl.c +++ b/src/boot/bootctl.c @@ -159,10 +159,9 @@ static int get_file_version(int fd, char **v) { if (buf == MAP_FAILED) return log_error_errno(errno, "Failed to memory map EFI binary: %m"); - s = memmem(buf, st.st_size - 8, "#### LoaderInfo: ", 17); + s = mempmem_safe(buf, st.st_size - 8, "#### LoaderInfo: ", 17); if (!s) goto finish; - s += 17; e = memmem(s, st.st_size - (s - buf), " ####", 5); if (!e || e - s < 3) { From e8b08edcdf4e3f22be0a209cacb9e5404fee4b68 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 10 Aug 2021 14:39:57 +0200 Subject: [PATCH 2/2] tree-wide: use memmem_safe() Let's be paranoid and do something useful if we operate with empty haystack/needle. This doesn't actually fix anything, as the places as far as I can see check for non-emptyness already beforehand, but I will sleep safer at night, if we don't even allow the trap to be fallen in, ever, even if the code is changed sooner or later. --- src/boot/bootctl.c | 2 +- src/import/pull-common.c | 17 +++++++++-------- src/libsystemd/sd-bus/bus-socket.c | 6 +++--- 3 files changed, 13 insertions(+), 12 deletions(-) diff --git a/src/boot/bootctl.c b/src/boot/bootctl.c index 9283e2013c..b1a0e313ce 100644 --- a/src/boot/bootctl.c +++ b/src/boot/bootctl.c @@ -163,7 +163,7 @@ static int get_file_version(int fd, char **v) { if (!s) goto finish; - e = memmem(s, st.st_size - (s - buf), " ####", 5); + e = memmem_safe(s, st.st_size - (s - buf), " ####", 5); if (!e || e - s < 3) { r = log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Malformed version string."); goto finish; diff --git a/src/import/pull-common.c b/src/import/pull-common.c index cb77454e0f..e1cd495102 100644 --- a/src/import/pull-common.c +++ b/src/import/pull-common.c @@ -10,6 +10,7 @@ #include "escape.h" #include "fd-util.h" #include "io-util.h" +#include "memory-util.h" #include "path-util.h" #include "process-util.h" #include "pull-common.h" @@ -342,18 +343,18 @@ static int verify_one(PullJob *checksum_job, PullJob *job) { line = strjoina(job->checksum, " *", fn, "\n"); - p = memmem(checksum_job->payload, - checksum_job->payload_size, - line, - strlen(line)); + p = memmem_safe(checksum_job->payload, + checksum_job->payload_size, + line, + strlen(line)); if (!p) { line = strjoina(job->checksum, " ", fn, "\n"); - p = memmem(checksum_job->payload, - checksum_job->payload_size, - line, - strlen(line)); + p = memmem_safe(checksum_job->payload, + checksum_job->payload_size, + line, + strlen(line)); } if (!p || (p != (char*) checksum_job->payload && p[-1] != '\n')) diff --git a/src/libsystemd/sd-bus/bus-socket.c b/src/libsystemd/sd-bus/bus-socket.c index b408d657a5..4b8d73c3e0 100644 --- a/src/libsystemd/sd-bus/bus-socket.c +++ b/src/libsystemd/sd-bus/bus-socket.c @@ -173,12 +173,12 @@ static int bus_socket_auth_verify_client(sd_bus *b) { if (!d) return 0; - e = memmem(d + 2, b->rbuffer_size - (d - (char*) b->rbuffer) - 2, "\r\n", 2); + e = memmem_safe(d + 2, b->rbuffer_size - (d - (char*) b->rbuffer) - 2, "\r\n", 2); if (!e) return 0; if (b->accept_fd) { - f = memmem(e + 2, b->rbuffer_size - (e - (char*) b->rbuffer) - 2, "\r\n", 2); + f = memmem_safe(e + 2, b->rbuffer_size - (e - (char*) b->rbuffer) - 2, "\r\n", 2); if (!f) return 0; @@ -399,7 +399,7 @@ static int bus_socket_auth_verify_server(sd_bus *b) { for (;;) { /* Check if line is complete */ line = (char*) b->rbuffer + b->auth_rbegin; - e = memmem(line, b->rbuffer_size - b->auth_rbegin, "\r\n", 2); + e = memmem_safe(line, b->rbuffer_size - b->auth_rbegin, "\r\n", 2); if (!e) return processed;