diff --git a/modulemd/include/modulemd-2.0/modulemd-module-index.h b/modulemd/include/modulemd-2.0/modulemd-module-index.h index cb876e89..a2fb679d 100644 --- a/modulemd/include/modulemd-2.0/modulemd-module-index.h +++ b/modulemd/include/modulemd-2.0/modulemd-module-index.h @@ -161,6 +161,12 @@ modulemd_module_index_new (void); * %NULL (if you don't care) or a pointer to %NULL (if you want to know the * error). On output, it will become allocated only if an error occured. * + * If libmodulemd is built with RPM I/O support, the YAML file might be + * compressed with a compression algorithm supported by RPM I/O. Then + * a content of the YAML file will be transparently decompressed. To prevent + * compression bomb attacks, a limit is imposed on the decompressed size: + * 64 MiB for small files and 64 times the compressed size for large files. + * * Returns: %TRUE if the update was successful. Returns %FALSE and sets * @failures appropriately if any of the YAML subdocuments were invalid or * sets @error if there was a fatal parse error. diff --git a/modulemd/include/private/modulemd-compression-private.h b/modulemd/include/private/modulemd-compression-private.h index ef6bdf65..1d8c7618 100644 --- a/modulemd/include/private/modulemd-compression-private.h +++ b/modulemd/include/private/modulemd-compression-private.h @@ -32,6 +32,26 @@ #ifdef HAVE_RPMIO +/** + * modulemd_decompressor_context: + * @rpmio_fd: RPM I/O file descriptor. + * @decompressed: How many decompressed bytes have been read so far. + * @limit: Maximum number of bytes allowed to be decompressed. + * # + * #modulemd_decompressor_context wraps file descriptor and axuliary metadata + * about a file to be decompressed. + * + * Since 2.15.4 + */ +typedef struct _modulemd_decompressor_context +{ + FD_t rpmio_fd; + size_t decompressed; + size_t limit; + +} modulemd_decompressor_context; + + /** * mmd_Fclose: * @fd: (in): A FD_t from rpmio.h to be closed diff --git a/modulemd/modulemd-compression.c b/modulemd/modulemd-compression.c index f5bf48b9..3c4ea789 100644 --- a/modulemd/modulemd-compression.c +++ b/modulemd/modulemd-compression.c @@ -20,6 +20,7 @@ #ifdef HAVE_RPMIO #include +#include #endif #include "modulemd-compression.h" @@ -251,17 +252,36 @@ compressed_stream_read_fn (void *data, size_t size, size_t *size_read) { - FD_t rpmio_fd = (FD_t)data; - ssize_t read = Fread (buffer, sizeof (*buffer), size, rpmio_fd); + struct _modulemd_decompressor_context *context = + (struct _modulemd_decompressor_context *)data; + ssize_t read = Fread (buffer, sizeof (*buffer), size, context->rpmio_fd); if (read < 0) { - g_warning ("Got error [%d] reading the file", Ferror (rpmio_fd)); + g_warning ("Got error [%d] reading the file", + Ferror (context->rpmio_fd)); return 0; } + context->decompressed += read; + if (context->decompressed < (size_t)read) + { + /* Handle integer overflow */ + context->decompressed = SIZE_MAX; + } - *size_read = read; + if (context->decompressed > context->limit) + { + g_warning ( + "A safety limit (%zu B) exceeded while decompressing the file", + context->limit); + // TODO: Store this reason into the context and pick it up from there + // whenever libmodulemd sets "Parser error" GError message after + // returning from yaml_parser_parse(). libyaml sets + // parser.error=YAML_READER_ERROR on any error from the reader callback. + return 0; + } + *size_read = read; return 1; } #else diff --git a/modulemd/modulemd-module-index.c b/modulemd/modulemd-module-index.c index ee5597ef..2532713b 100644 --- a/modulemd/modulemd-module-index.c +++ b/modulemd/modulemd-module-index.c @@ -19,6 +19,8 @@ #ifdef HAVE_RPMIO #include +#include +#include #endif #include "modulemd.h" @@ -641,8 +643,24 @@ modulemd_module_index_update_from_file_ext (ModulemdModuleIndex *self, * suite of tools to deal with it. We need to construct a special "mode" * argument to pass to Fdopen(). */ - FD_t rpmio_fd = NULL; + modulemd_decompressor_context context = { + .rpmio_fd = NULL, .decompressed = 0, .limit = 1 << 26, /* 64 MB */ + }; g_auto (FD_t) fd_dup = NULL; + struct stat statbuf; + + /* Scale the decompression limit according to a compressed size. + * A compression ratio of XZ on typical modulemd data is 1:30. + * Giving a leeway of factor 2 and rounding it to a power of 2 yields + * a factor limit of 64 (2^6). */ + if (0 == fstat (fd, &statbuf) && statbuf.st_size > 0) + { + size_t scaled_limit = (statbuf.st_size <= SIZE_MAX >> 6) ? + (size_t)statbuf.st_size << 6 : + SIZE_MAX; + if (scaled_limit > context.limit) + context.limit = scaled_limit; + } fmode = modulemd_get_rpmio_fmode ("r", comtype); if (!fmode) @@ -674,9 +692,9 @@ modulemd_module_index_update_from_file_ext (ModulemdModuleIndex *self, } g_debug ("Calling rpmio::Fdopen (%p, %s)", fd_dup, fmode); - rpmio_fd = Fdopen (fd_dup, fmode); + context.rpmio_fd = Fdopen (fd_dup, fmode); - if (!rpmio_fd) + if (!context.rpmio_fd) { g_set_error_literal ( error, @@ -689,7 +707,7 @@ modulemd_module_index_update_from_file_ext (ModulemdModuleIndex *self, g_debug ("rpmio::Fdopen (%p, %s) succeeded", fd_dup, fmode); return modulemd_module_index_update_from_custom ( - self, compressed_stream_read_fn, rpmio_fd, strict, failures, error); + self, compressed_stream_read_fn, &context, strict, failures, error); #else /* HAVE_RPMIO */ g_set_error_literal ( diff --git a/modulemd/tests/test-modulemd-compression.c b/modulemd/tests/test-modulemd-compression.c index 06b3c618..ce834058 100644 --- a/modulemd/tests/test-modulemd-compression.c +++ b/modulemd/tests/test-modulemd-compression.c @@ -14,7 +14,11 @@ #include #include +#include "config.h" #include "modulemd-compression.h" +#ifdef HAVE_RPMIO +#include "modulemd-module-index.h" +#endif #include "private/modulemd-compression-private.h" #include "private/modulemd-yaml.h" #include "private/test-utils.h" @@ -224,6 +228,40 @@ test_modulemd_get_rpmio_fmode (void) } +#ifdef HAVE_RPMIO +/* A compressed file smaller than 64 MiB / 64 which decompresses to + * more than 64 MiB should be rejected. */ +static void +test_modulemd_minimal_limit (void) +{ + if (g_test_subprocess ()) + { + g_autofree const gchar *filename = NULL; + g_autoptr (ModulemdModuleIndex) index = NULL; + g_autoptr (GPtrArray) failures = NULL; + g_autoptr (GError) error = NULL; + + filename = g_strdup_printf ("%s/compression/small_bomb.yaml.gz", + g_getenv ("TEST_DATA_PATH")); + g_assert_nonnull (filename); + index = modulemd_module_index_new (); + g_assert_nonnull (index); + modulemd_module_index_update_from_file ( + index, filename, TRUE, &failures, &error); + /* TODO: Check for false return value and set error, once libmodulemd + * supports reporting YAML reader callback errors. Until then we need + * to wrap this modulemd_module_index_update_from_file() call into + * a subprocess because the "safety limit" warning in the callback + * aborts the tests when G_DEBUG=fatal-warning environment variable is + * set. */ + return; + } + g_test_trap_subprocess (NULL, 0, G_TEST_SUBPROCESS_DEFAULT); + g_test_trap_assert_failed (); +} +#endif + + int main (int argc, char *argv[]) { @@ -244,5 +282,10 @@ main (int argc, char *argv[]) g_test_add_func ("/modulemd/compression/rmpio/fmode", test_modulemd_get_rpmio_fmode); +#ifdef HAVE_RPMIO + g_test_add_func ("/modulemd/compression/limit/minimum", + test_modulemd_minimal_limit); +#endif + return g_test_run (); } diff --git a/modulemd/tests/test_data/compression/small_bomb.yaml.gz b/modulemd/tests/test_data/compression/small_bomb.yaml.gz new file mode 100644 index 00000000..220ad6d3 Binary files /dev/null and b/modulemd/tests/test_data/compression/small_bomb.yaml.gz differ