Attachment 88787 Details for Bug 71060 – Check that read() succeeds

[patch] Check that read() succeeds

check-result-from-read.patch (text/plain), 3.88 KB, created by Uli Schlachter on 2013-11-06 21:50:55 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Uli Schlachter

Created: 2013-11-06 21:50:55 UTC

Size: 3.88 KB

patch

obsolete

>diff --git a/cursor/parse_cursor_file.c b/cursor/parse_cursor_file.c
>index 30f7df5..ea75e88 100644
>--- a/cursor/parse_cursor_file.c
>+++ b/cursor/parse_cursor_file.c
>@@ -82,20 +82,25 @@ static uint32_t find_best_size(xcint_cursor_file_t *cf, const uint32_t target, u
>     return best;
> }
> 
>+static int try_read(int fd, void *buf, size_t count) {
>+    return read(fd, buf, count) == count;
>+}
>+
> int parse_cursor_file(xcb_cursor_context_t *c, const int fd, xcint_image_t **images, int *nimg) {
>     /* Read the header, verify the magic value. */
>     xcint_cursor_file_t cf;
>     uint32_t nsizes = 0;
>     uint32_t best = 0;
>     uint32_t skip = 0;
>+    int success;
> 
>-    read(fd, &(cf.header), sizeof(xcint_file_header_t));
>+    success = try_read(fd, &(cf.header), sizeof(xcint_file_header_t));
>     cf.header.magic = le32toh(cf.header.magic);
>     cf.header.header = le32toh(cf.header.header);
>     cf.header.version = le32toh(cf.header.version);
>     cf.header.ntoc = le32toh(cf.header.ntoc);
> 
>-    if (cf.header.magic != XCURSOR_MAGIC)
>+    if (!success || cf.header.magic != XCURSOR_MAGIC)
>         return -EINVAL;
> 
>     if ((skip = (cf.header.header - sizeof(xcint_file_header_t))) > 0)
>@@ -107,7 +112,9 @@ int parse_cursor_file(xcb_cursor_context_t *c, const int fd, xcint_image_t **ima
> 
>     /* Read the table of contents */
>     cf.tocs = malloc(cf.header.ntoc * sizeof(xcint_file_toc_t));
>-    read(fd, cf.tocs, cf.header.ntoc * sizeof(xcint_file_toc_t));
>+    success = try_read(fd, cf.tocs, cf.header.ntoc * sizeof(xcint_file_toc_t));
>+    if (!success)
>+        return -EINVAL;
>     for (int n = 0; n < cf.header.ntoc; n++) {
>         cf.tocs[n].type = le32toh(cf.tocs[n].type);
>         cf.tocs[n].subtype = le32toh(cf.tocs[n].subtype);
>@@ -140,18 +147,19 @@ int parse_cursor_file(xcb_cursor_context_t *c, const int fd, xcint_image_t **ima
>             continue;
> 
>         lseek(fd, cf.tocs[n].position, SEEK_SET);
>-        read(fd, &chunk, sizeof(xcint_chunk_header_t));
>+        success = try_read(fd, &chunk, sizeof(xcint_chunk_header_t));
>         chunk.header = le32toh(chunk.header);
>         chunk.type = le32toh(chunk.type);
>         chunk.subtype = le32toh(chunk.subtype);
>         chunk.version = le32toh(chunk.version);
>         /* Sanity check, as libxcursor does it. */
>-        if (chunk.type != cf.tocs[n].type ||
>+        if (!success ||
>+            chunk.type != cf.tocs[n].type ||
>             chunk.subtype != cf.tocs[n].subtype) {
>             free(cf.tocs);
>             return -EINVAL;
>         }
>-        read(fd, i, sizeof(xcint_image_t) - sizeof(uint32_t*)); // TODO: better type
>+        success = try_read(fd, i, sizeof(xcint_image_t) - sizeof(uint32_t*)); // TODO: better type
>         i->width = le32toh(i->width);
>         i->height = le32toh(i->height);
>         i->xhot = le32toh(i->xhot);
>@@ -159,14 +167,21 @@ int parse_cursor_file(xcb_cursor_context_t *c, const int fd, xcint_image_t **ima
>         i->delay = le32toh(i->delay);
> 
>         /* Read the actual image data and convert it to host byte order */
>-        if (((uint64_t)i->width) * i->height > UINT32_MAX) {
>+        if (!success || ((uint64_t)i->width) * i->height > UINT32_MAX) {
>             /* Catch integer overflows */
>             free(cf.tocs);
>+            /* XXX: This leaks images[i]->pixels for all other images (and leaks 'images' itself) */
>             return -EINVAL;
>         }
>         numpixels = i->width * i->height;
>         i->pixels = malloc(numpixels * sizeof(uint32_t));
>-        read(fd, i->pixels, numpixels * sizeof(uint32_t));
>+        success = try_read(fd, i->pixels, numpixels * sizeof(uint32_t));
>+        if (!success) {
>+            free(i->pixels);
>+            free(cf.tocs);
>+            /* XXX: This leaks images[i]->pixels for all other images (and leaks 'images' itself) */
>+            return -EINVAL;
>+        }
>         p = i->pixels;
>         for (int j = 0; j < numpixels; j++, p++)
>             *p = le32toh(*p);

Actions: View | Splinter Review

Attachments on bug 71060: 88374 | 88787 | 88788 | 88910 | 88928

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.