1
0
mirror of https://github.com/systemd/systemd.git synced 2025-03-09 12:58:26 +03:00

tree-wide: check memstream buffer after closing the handle

When closing the FILE handle attached to a memstream, it may attempt to
do a realloc() that may fail during OOM situations, in which case we are
left with the buffer pointer pointing to NULL and buffer size > 0. For
example:

```
    #include <errno.h>
    #include <stdio.h>
    #include <stdlib.h>

    void *realloc(void *ptr, size_t size) {
        return NULL;
    }

    int main(int argc, char *argv[])
    {
        FILE *f;
        char *buf;
        size_t sz = 0;

        f = open_memstream(&buf, &sz);
        if (!f)
            return -ENOMEM;

        fputs("Hello", f);

        fflush(f);
        printf("buf: 0x%lx, sz: %lu, errno: %d\n",
                    (unsigned long) buf, sz, errno);
        fclose(f);
        printf("buf: 0x%lx, sz: %lu, errno: %d\n",
                    (unsigned long) buf, sz, errno);

        return 0;
    }
```

```
$ gcc -o main main.c
$ ./main
buf: 0x74d4a0, sz: 5, errno: 0
buf: 0x0, sz: 5, errno: 0
```

This might do unexpected things if the underlying code expects a valid
pointer to the memstream buffer after closing the handle.

Found by Nallocfuzz.
This commit is contained in:
Frantisek Sumsal 2023-05-24 13:29:52 +02:00
parent 08a8fd6e8d
commit f392dfb5a1
17 changed files with 132 additions and 70 deletions

View File

@ -9,6 +9,7 @@
#include "alloc-util.h"
#include "escape.h"
#include "extract-word.h"
#include "fd-util.h"
#include "fileio.h"
#include "gunicode.h"
#include "locale-util.h"
@ -603,9 +604,9 @@ char *strip_tab_ansi(char **ibuf, size_t *_isz, size_t highlight[2]) {
STATE_CSI,
STATE_CSO,
} state = STATE_OTHER;
char *obuf = NULL;
_cleanup_free_ char *obuf = NULL;
_cleanup_fclose_ FILE *f = NULL;
size_t osz = 0, isz, shift[2] = {}, n_carriage_returns = 0;
FILE *f;
assert(ibuf);
assert(*ibuf);
@ -713,11 +714,13 @@ char *strip_tab_ansi(char **ibuf, size_t *_isz, size_t highlight[2]) {
}
}
if (fflush_and_check(f) < 0) {
fclose(f);
return mfree(obuf);
}
fclose(f);
if (fflush_and_check(f) < 0)
return NULL;
f = safe_fclose(f);
if (!obuf)
return NULL;
free_and_replace(*ibuf, obuf);

View File

@ -1072,6 +1072,9 @@ static int introspect(int argc, char **argv, void *userdata) {
mf = safe_fclose(mf);
if (!buf)
return bus_log_parse_error(ENOMEM);
z = set_get(members, &((Member) {
.type = "property",
.interface = m->interface,

View File

@ -95,6 +95,9 @@ int manager_get_dump_string(Manager *m, char **patterns, char **ret) {
f = safe_fclose(f);
if (!dump)
return -ENOMEM;
*ret = TAKE_PTR(dump);
return 0;

View File

@ -707,6 +707,9 @@ static int compose_open_fds(pid_t pid, char **open_fds) {
if (errno > 0)
return -errno;
if (!buffer)
return -ENOMEM;
*open_fds = TAKE_PTR(buffer);
return 0;

View File

@ -1811,6 +1811,10 @@ static int format_journal_url(
return r;
f = safe_fclose(f);
if (!url)
return -ENOMEM;
*ret_url = TAKE_PTR(url);
return 0;
}

View File

@ -268,6 +268,10 @@ int introspect_finish(struct introspect *i, char **ret) {
return r;
i->f = safe_fclose(i->f);
if (!i->introspection)
return -ENOMEM;
*ret = TAKE_PTR(i->introspection);
return 0;

View File

@ -824,6 +824,7 @@ int bus_match_parse(
char *bus_match_to_string(struct bus_match_component *components, size_t n_components) {
_cleanup_free_ char *buffer = NULL;
_cleanup_fclose_ FILE *f = NULL;
size_t size = 0;
int r;
@ -832,7 +833,7 @@ char *bus_match_to_string(struct bus_match_component *components, size_t n_compo
assert(components);
FILE *f = open_memstream_unlocked(&buffer, &size);
f = open_memstream_unlocked(&buffer, &size);
if (!f)
return NULL;
@ -855,9 +856,11 @@ char *bus_match_to_string(struct bus_match_component *components, size_t n_compo
}
r = fflush_and_check(f);
safe_fclose(f);
if (r < 0)
return NULL;
f = safe_fclose(f);
return TAKE_PTR(buffer);
}

View File

@ -1199,30 +1199,31 @@ void link_dump(Link *link, FILE *f) {
int network_format(Network *network, char **ret) {
_cleanup_free_ char *s = NULL;
_cleanup_fclose_ FILE *f = NULL;
size_t sz = 0;
int r;
assert(network);
assert(ret);
{
_cleanup_fclose_ FILE *f = NULL;
f = open_memstream_unlocked(&s, &sz);
if (!f)
return -ENOMEM;
f = open_memstream_unlocked(&s, &sz);
if (!f)
return -ENOMEM;
network_dump(network, f);
network_dump(network, f);
/* Add terminating 0, so that the output buffer is a valid string. */
fputc('\0', f);
/* Add terminating 0, so that the output buffer is a valid string. */
fputc('\0', f);
r = fflush_and_check(f);
}
r = fflush_and_check(f);
if (r < 0)
return r;
assert(s);
f = safe_fclose(f);
if (!s)
return -ENOMEM;
*ret = TAKE_PTR(s);
assert(sz > 0);
return (int) sz - 1;
@ -1230,30 +1231,31 @@ int network_format(Network *network, char **ret) {
int netdev_format(NetDev *netdev, char **ret) {
_cleanup_free_ char *s = NULL;
_cleanup_fclose_ FILE *f = NULL;
size_t sz = 0;
int r;
assert(netdev);
assert(ret);
{
_cleanup_fclose_ FILE *f = NULL;
f = open_memstream_unlocked(&s, &sz);
if (!f)
return -ENOMEM;
f = open_memstream_unlocked(&s, &sz);
if (!f)
return -ENOMEM;
netdev_dump(netdev, f);
netdev_dump(netdev, f);
/* Add terminating 0, so that the output buffer is a valid string. */
fputc('\0', f);
/* Add terminating 0, so that the output buffer is a valid string. */
fputc('\0', f);
r = fflush_and_check(f);
}
r = fflush_and_check(f);
if (r < 0)
return r;
assert(s);
f = safe_fclose(f);
if (!s)
return -ENOMEM;
*ret = TAKE_PTR(s);
assert(sz > 0);
return (int) sz - 1;
@ -1261,30 +1263,31 @@ int netdev_format(NetDev *netdev, char **ret) {
int link_format(Link *link, char **ret) {
_cleanup_free_ char *s = NULL;
_cleanup_fclose_ FILE *f = NULL;
size_t sz = 0;
int r;
assert(link);
assert(ret);
{
_cleanup_fclose_ FILE *f = NULL;
f = open_memstream_unlocked(&s, &sz);
if (!f)
return -ENOMEM;
f = open_memstream_unlocked(&s, &sz);
if (!f)
return -ENOMEM;
link_dump(link, f);
link_dump(link, f);
/* Add terminating 0, so that the output buffer is a valid string. */
fputc('\0', f);
/* Add terminating 0, so that the output buffer is a valid string. */
fputc('\0', f);
r = fflush_and_check(f);
}
r = fflush_and_check(f);
if (r < 0)
return r;
assert(s);
f = safe_fclose(f);
if (!s)
return -ENOMEM;
*ret = TAKE_PTR(s);
assert(sz > 0);
return (int) sz - 1;

View File

@ -847,6 +847,9 @@ int manager_get_dump_string(Manager *m, char **ret) {
f = safe_fclose(f);
if (!dump)
return -ENOMEM;
*ret = TAKE_PTR(dump);
return 0;
}

View File

@ -299,6 +299,11 @@ static int dump_kill_candidates(OomdCGroupContext **sorted, int n, int dump_unti
if (r < 0)
return r;
f = safe_fclose(f);
if (!dump)
return -ENOMEM;
return log_dump(LOG_INFO, dump);
}

View File

@ -867,10 +867,14 @@ static int dnssec_rrset_serialize_sig(
}
r = fflush_and_check(f);
f = safe_fclose(f); /* sig_data may be reallocated when f is closed. */
if (r < 0)
return r;
f = safe_fclose(f); /* sig_data may be reallocated when f is closed. */
if (!sig_data)
return -ENOMEM;
*ret_sig_data = TAKE_PTR(sig_data);
*ret_sig_size = sig_size;
return 0;

View File

@ -519,6 +519,11 @@ static int manager_sigusr1(sd_event_source *s, const struct signalfd_siginfo *si
if (fflush_and_check(f) < 0)
return log_oom();
f = safe_fclose(f);
if (!buffer)
return -ENOMEM;
log_dump(LOG_INFO, buffer);
return 0;
}

View File

@ -628,6 +628,9 @@ static int method_dump_memory_state_by_fd(sd_bus_message *message, void *userdat
dump_file = safe_fclose(dump_file);
if (!dump)
return -ENOMEM;
fd = acquire_data_fd(dump, dump_size, 0);
if (fd < 0)
return fd;

View File

@ -12,6 +12,7 @@
#include "alloc-util.h"
#include "calendarspec.h"
#include "errno-util.h"
#include "fd-util.h"
#include "fileio.h"
#include "macro.h"
#include "parse-util.h"
@ -337,9 +338,9 @@ static void format_chain(FILE *f, int space, const CalendarComponent *c, bool us
}
int calendar_spec_to_string(const CalendarSpec *c, char **p) {
char *buf = NULL;
_cleanup_free_ char *buf = NULL;
_cleanup_fclose_ FILE *f = NULL;
size_t sz = 0;
FILE *f;
int r;
assert(c);
@ -384,14 +385,15 @@ int calendar_spec_to_string(const CalendarSpec *c, char **p) {
}
r = fflush_and_check(f);
fclose(f);
if (r < 0) {
free(buf);
if (r < 0)
return r;
}
*p = buf;
f = safe_fclose(f);
if (!buf)
return -ENOMEM;
*p = TAKE_PTR(buf);
return 0;
}

View File

@ -621,8 +621,13 @@ static int parse_core(int fd, const char *executable, char **ret, JsonVariant **
return log_warning_errno(r, "Could not parse core file, flushing file buffer failed: %m");
c.f = safe_fclose(c.f);
if (!buf)
return log_oom();
*ret = TAKE_PTR(buf);
}
if (ret_package_metadata)
*ret_package_metadata = TAKE_PTR(package_metadata);
@ -735,8 +740,13 @@ static int parse_elf(int fd, const char *executable, char **ret, JsonVariant **r
return log_warning_errno(r, "Could not parse ELF file, flushing file buffer failed: %m");
c.f = safe_fclose(c.f);
if (!buf)
return log_oom();
*ret = TAKE_PTR(buf);
}
if (ret_package_metadata)
*ret_package_metadata = TAKE_PTR(elf_metadata);

View File

@ -2537,6 +2537,9 @@ int table_format(Table *t, char **ret) {
f = safe_fclose(f);
if (!buf)
return -ENOMEM;
*ret = TAKE_PTR(buf);
return 0;

View File

@ -1769,6 +1769,7 @@ static int json_format(FILE *f, JsonVariant *v, JsonFormatFlags flags, const cha
int json_variant_format(JsonVariant *v, JsonFormatFlags flags, char **ret) {
_cleanup_free_ char *s = NULL;
_cleanup_fclose_ FILE *f = NULL;
size_t sz = 0;
int r;
@ -1781,26 +1782,26 @@ int json_variant_format(JsonVariant *v, JsonFormatFlags flags, char **ret) {
if (flags & JSON_FORMAT_OFF)
return -ENOEXEC;
{
_cleanup_fclose_ FILE *f = NULL;
f = open_memstream_unlocked(&s, &sz);
if (!f)
return -ENOMEM;
f = open_memstream_unlocked(&s, &sz);
if (!f)
return -ENOMEM;
r = json_variant_dump(v, flags, f, NULL);
if (r < 0)
return r;
/* Add terminating 0, so that the output buffer is a valid string. */
fputc('\0', f);
r = fflush_and_check(f);
}
r = json_variant_dump(v, flags, f, NULL);
if (r < 0)
return r;
assert(s);
/* Add terminating 0, so that the output buffer is a valid string. */
fputc('\0', f);
r = fflush_and_check(f);
if (r < 0)
return r;
f = safe_fclose(f);
if (!s)
return -ENOMEM;
*ret = TAKE_PTR(s);
assert(sz > 0);
return (int) sz - 1;