Improve error handling of (de)compressors

Previously, some kinds of corruption were not detected by the zlib
decompressor since it didn’t check that it had reached the end of the
stream and therefore didn’t verify the Adler-32 checksum.

diff --git a/src/ccache.c b/src/ccache.c
index 903c4b42824790ea6ebda8188f36de02cc1b66df..4370b6e1273d5f7932127ac26639633e2da58e24 100644 (file)
--- a/src/ccache.c
+++ b/src/ccache.c
@@ -3889,7 +3889,10 @@ ccache_main_options(int argc, char *argv[])
 
                case DUMP_RESULT:
                        initialize();
-                       cache_dump(optarg, stdout);
+                       if (!cache_dump(optarg, stdout)) {
+                               fprintf(stderr, "Error: Corrupt result file\n");
+                               return 1;
+                       }
                        break;
 
                case HASH_FILE:

diff --git a/src/compr_none.c b/src/compr_none.c
index d16708d07ca6edebe22efa548f60fa99700dd868..2aec2a22bacbafc553b8e1038ccd1bb89820e42e 100644 (file)
--- a/src/compr_none.c
+++ b/src/compr_none.c
@@ -30,10 +30,11 @@ compr_none_write(struct compr_state *handle, const void *data, size_t size)
        return fwrite(data, 1, size, output) == size;
 }
 
-static void
+static bool
 compr_none_free(struct compr_state *handle)
 {
-       (void)handle;
+       FILE *output = (FILE *)handle;
+       return ferror(output) == 0;
 }
 
 struct compressor compr_none = {

diff --git a/src/compr_zlib.c b/src/compr_zlib.c
index 0462d50fea9248e819daf591da7a2e9e576d119b..83cc28e79e351a5e70ce646fe13ef5b0505fa1d7 100644 (file)
--- a/src/compr_zlib.c
+++ b/src/compr_zlib.c
@@ -23,6 +23,7 @@ struct state
 {
        FILE *output;
        z_stream stream;
+       bool failed;
 };
 
 static struct compr_state *
@@ -33,6 +34,7 @@ compr_zlib_init(FILE *output, int level)
        state->stream.zalloc = Z_NULL;
        state->stream.zfree = Z_NULL;
        state->stream.opaque = Z_NULL;
+       state->failed = false;
 
        int ret = deflateInit(&state->stream, level);
        if (ret != Z_OK) {
@@ -45,6 +47,9 @@ compr_zlib_init(FILE *output, int level)
 static bool
 compr_zlib_write(struct compr_state *handle, const void *data, size_t size)
 {
+       if (!handle) {
+               return false;
+       }
        struct state *state = (struct state *)handle;
 
        state->stream.next_in = (const Bytef *)data;
@@ -62,6 +67,7 @@ compr_zlib_write(struct compr_state *handle, const void *data, size_t size)
                unsigned int compressed_bytes = sizeof(buffer) - state->stream.avail_out;
                if (fwrite(buffer, 1, compressed_bytes, state->output) != compressed_bytes
                    || ferror(state->output)) {
+                       state->failed = true;
                        return false;
                }
        } while (state->stream.avail_out == 0);
@@ -71,14 +77,19 @@ compr_zlib_write(struct compr_state *handle, const void *data, size_t size)
        return true;
 }
 
-static void
+static bool
 compr_zlib_free(struct compr_state *handle)
 {
+       if (!handle) {
+               return false;
+       }
        struct state *state = (struct state *)handle;
 
        compr_zlib_write(handle, NULL, 0);
        deflateEnd(&state->stream);
+       bool success = !state->failed;
        free(state);
+       return success;
 }
 
 struct compressor compr_zlib = {

diff --git a/src/compression.h b/src/compression.h
index a48ab03217fd5384a4e087705af4b23f4ac0b1bb..89bf1469f730d0db82650dca61dc00b74e605869 100644 (file)
--- a/src/compression.h
+++ b/src/compression.h
@@ -8,7 +8,7 @@ struct compr_state;
 struct compressor {
        struct compr_state *(*init)(FILE *output, int compression_level);
        bool (*write)(struct compr_state *state, const void *data, size_t size);
-       void (*free)(struct compr_state *state);
+       bool (*free)(struct compr_state *state);
 };
 
 struct decompr_state;
@@ -16,7 +16,7 @@ struct decompr_state;
 struct decompressor {
        struct decompr_state *(*init)(FILE *input);
        bool (*read)(struct decompr_state *state, void *data, size_t size);
-       void (*free)(struct decompr_state *state);
+       bool (*free)(struct decompr_state *state);
 };
 
 extern struct compressor compr_none;

diff --git a/src/decompr_none.c b/src/decompr_none.c
index aa96f0623e0697af10c29053f09ec046f6f2b773..4087bb95748f396f2a6cc41dcbaf7232de999183 100644 (file)
--- a/src/decompr_none.c
+++ b/src/decompr_none.c
@@ -29,9 +29,11 @@ decompr_none_read(struct decompr_state *handle, void *data, size_t size)
        return fread(data, 1, size, input) == size;
 }
 
-static void decompr_none_free(struct decompr_state *handle)
+static bool
+decompr_none_free(struct decompr_state *handle)
 {
-       (void)handle;
+       FILE *input = (FILE *)handle;
+       return ferror(input) == 0;
 }
 
 struct decompressor decompr_none = {

diff --git a/src/decompr_zlib.c b/src/decompr_zlib.c
index 3c97e67e317ae25f71bae4812ef30286f16a9eb2..6967f66dd69c534c363585e0fd92987f8ae39521 100644 (file)
--- a/src/decompr_zlib.c
+++ b/src/decompr_zlib.c
@@ -19,6 +19,12 @@
 
 #include <zlib.h>
 
+enum stream_state {
+       STREAM_STATE_READING,
+       STREAM_STATE_FAILED,
+       STREAM_STATE_END
+};
+
 struct state
 {
        FILE *input;
@@ -26,6 +32,7 @@ struct state
        size_t input_size;
        size_t input_consumed;
        z_stream stream;
+       enum stream_state stream_state;
 };
 
 static struct decompr_state *
@@ -41,6 +48,7 @@ decompr_zlib_init(FILE *input)
        state->stream.opaque = Z_NULL;
        state->stream.avail_in = 0;
        state->stream.next_in = Z_NULL;
+       state->stream_state = STREAM_STATE_READING;
 
        int ret = inflateInit(&state->stream);
        if (ret != Z_OK) {
@@ -53,6 +61,9 @@ decompr_zlib_init(FILE *input)
 static bool
 decompr_zlib_read(struct decompr_state *handle, void *data, size_t size)
 {
+       if (!handle) {
+               return false;
+       }
        struct state *state = (struct state *)handle;
 
        size_t bytes_read = 0;
@@ -62,6 +73,7 @@ decompr_zlib_read(struct decompr_state *handle, void *data, size_t size)
                        state->input_size = fread(
                                state->input_buffer, 1, sizeof(state->input_buffer), state->input);
                        if (state->input_size == 0) {
+                               state->stream_state = STREAM_STATE_FAILED;
                                return false;
                        }
                        state->input_consumed = 0;
@@ -79,7 +91,11 @@ decompr_zlib_read(struct decompr_state *handle, void *data, size_t size)
                case Z_NEED_DICT:
                case Z_DATA_ERROR:
                case Z_MEM_ERROR:
+                       state->stream_state = STREAM_STATE_FAILED;
                        return false;
+               case Z_STREAM_END:
+                       state->stream_state = STREAM_STATE_END;
+                       break;
                }
                bytes_read = size - state->stream.avail_out;
                state->input_consumed = state->input_size - state->stream.avail_in;
@@ -88,11 +104,16 @@ decompr_zlib_read(struct decompr_state *handle, void *data, size_t size)
        return true;
 }
 
-static void decompr_zlib_free(struct decompr_state *handle)
+static bool decompr_zlib_free(struct decompr_state *handle)
 {
+       if (!handle) {
+               return false;
+       }
        struct state *state = (struct state *)handle;
        inflateEnd(&state->stream);
+       bool success = state->stream_state == STREAM_STATE_END;
        free(handle);
+       return success;
 }
 
 struct decompressor decompr_zlib = {

diff --git a/src/result.c b/src/result.c
index 43db2b57a9ca647cc450c07117ce50d018f00507..c7cc5735c2287bd644bc8cc15a0e3cb3be5bfdfb 100644 (file)
--- a/src/result.c
+++ b/src/result.c
@@ -196,6 +196,10 @@ read_cache(const char *path, struct filelist *l, FILE *dump_stream)
        }
 
        decompr_state = decompressor->init(f);
+       if (!decompr_state) {
+               cc_log("Failed to initialize decompressor");
+               goto out;
+       }
 
        if (dump_stream) {
                const uint8_t compr_level = header[6];
@@ -287,8 +291,8 @@ out:
        if (subfile) {
                fclose(subfile);
        }
-       if (decompressor) {
-               decompressor->free(decompr_state);
+       if (decompressor && !decompressor->free(decompr_state)) {
+               success = false;
        }
        if (f) {
                fclose(f);
@@ -401,10 +405,13 @@ bool cache_put(const char *cache_path, struct filelist *l,
                compressor = &compr_zlib;
        }
 
-       struct compr_state *compr_state =
-               compressor->init(f, compression_level);
-       bool ok = write_cache(l, compressor, compr_state);
-       compressor->free(compr_state);
+       struct compr_state *compr_state = compressor->init(f, compression_level);
+       if (!compr_state) {
+               cc_log("Failed to initialize compressor");
+               goto out;
+       }
+       bool ok = write_cache(l, compressor, compr_state)
+               && compressor->free(compr_state);
        if (!ok) {
                cc_log("Failed to write cache file");
                goto out;

diff --git a/unittest/test_compr_zlib.c b/unittest/test_compr_zlib.c
index ebbab2ada7881e6a63bbc2f257511ae881012a3b..f5bc1a1427aed18aa420eba66327f497e31cf352 100644 (file)
--- a/unittest/test_compr_zlib.c
+++ b/unittest/test_compr_zlib.c
@@ -28,7 +28,7 @@ TEST(zlib_small_roundtrip)
 
        CHECK(compr_zlib.write(c_state, "foobar", 6));
 
-       compr_zlib.free(c_state);
+       CHECK(compr_zlib.free(c_state));
        fclose(f);
 
        f = fopen("data.zlib", "r");
@@ -44,7 +44,8 @@ TEST(zlib_small_roundtrip)
        // Nothing left to read.
        CHECK(!decompr_zlib.read(d_state, buffer, 1));
 
-       decompr_zlib.free(d_state);
+       // Error state is remembered.
+       CHECK(!decompr_zlib.free(d_state));
        fclose(f);
 }
 
@@ -60,7 +61,7 @@ TEST(zlib_large_compressible_roundtrip)
                CHECK(compr_zlib.write(c_state, data, sizeof(data)));
        }
 
-       compr_zlib.free(c_state);
+       CHECK(compr_zlib.free(c_state));
        fclose(f);
 
        f = fopen("data.zlib", "r");
@@ -76,7 +77,8 @@ TEST(zlib_large_compressible_roundtrip)
        // Nothing left to read.
        CHECK(!decompr_zlib.read(d_state, buffer, 1));
 
-       decompr_zlib.free(d_state);
+       // Error state is remembered.
+       CHECK(!decompr_zlib.free(d_state));
        fclose(f);
 }
 
@@ -93,7 +95,7 @@ TEST(zlib_large_uncompressible_roundtrip)
 
        CHECK(compr_zlib.write(c_state, data, sizeof(data)));
 
-       compr_zlib.free(c_state);
+       CHECK(compr_zlib.free(c_state));
        fclose(f);
 
        f = fopen("data.zlib", "r");
@@ -104,10 +106,7 @@ TEST(zlib_large_uncompressible_roundtrip)
        CHECK(decompr_zlib.read(d_state, buffer, sizeof(buffer)));
        CHECK(memcmp(buffer, data, sizeof(data)) == 0);
 
-       // Nothing left to read.
-       CHECK(!decompr_zlib.read(d_state, buffer, 1));
-
-       decompr_zlib.free(d_state);
+       CHECK(decompr_zlib.free(d_state));
        fclose(f);
 }