liblzma: Validate encoder arguments better.
authorLasse Collin <lasse.collin@tukaani.org>
Mon, 11 Apr 2011 10:21:28 +0000 (13:21 +0300)
committerLasse Collin <lasse.collin@tukaani.org>
Mon, 11 Apr 2011 10:21:28 +0000 (13:21 +0300)
The biggest problem was that the integrity check type
wasn't validated, and e.g. lzma_easy_buffer_encode()
would create a corrupt .xz Stream if given an unsupported
Check ID. Luckily applications don't usually try to use
an unsupport Check ID, so this bug is unlikely to cause
many real-world problems.

src/liblzma/common/block_buffer_encoder.c
src/liblzma/common/block_encoder.c
src/liblzma/common/stream_buffer_encoder.c

index a8f71c2..519c6a6 100644 (file)
@@ -226,16 +226,23 @@ lzma_block_buffer_encode(lzma_block *block, lzma_allocator *allocator,
                const uint8_t *in, size_t in_size,
                uint8_t *out, size_t *out_pos, size_t out_size)
 {
-       // Sanity checks
-       if (block == NULL || block->filters == NULL
-                       || (in == NULL && in_size != 0) || out == NULL
+       // Validate the arguments.
+       if (block == NULL || (in == NULL && in_size != 0) || out == NULL
                        || out_pos == NULL || *out_pos > out_size)
                return LZMA_PROG_ERROR;
 
-       // Check the version field.
+       // The contents of the structure may depend on the version so
+       // check the version before validating the contents of *block.
        if (block->version != 0)
                return LZMA_OPTIONS_ERROR;
 
+       if ((unsigned int)(block->check) > LZMA_CHECK_ID_MAX
+                       || block->filters == NULL)
+               return LZMA_PROG_ERROR;
+
+       if (!lzma_check_is_supported(block->check))
+               return LZMA_UNSUPPORTED_CHECK;
+
        // Size of a Block has to be a multiple of four, so limit the size
        // here already. This way we don't need to check it again when adding
        // Block Padding.
@@ -243,8 +250,7 @@ lzma_block_buffer_encode(lzma_block *block, lzma_allocator *allocator,
 
        // Get the size of the Check field.
        const size_t check_size = lzma_check_size(block->check);
-       if (check_size == UINT32_MAX)
-               return LZMA_PROG_ERROR;
+       assert(check_size != UINT32_MAX);
 
        // Reserve space for the Check field.
        if (out_size - *out_pos <= check_size)
index ca51523..b34c501 100644 (file)
@@ -161,6 +161,11 @@ lzma_block_encoder_init(lzma_next_coder *next, lzma_allocator *allocator,
 {
        lzma_next_coder_init(&lzma_block_encoder_init, next, allocator);
 
+       if (block == NULL)
+               return LZMA_PROG_ERROR;
+
+       // The contents of the structure may depend on the version so
+       // check the version first.
        if (block->version != 0)
                return LZMA_OPTIONS_ERROR;
 
index f727d85..dd23c9a 100644 (file)
@@ -51,6 +51,9 @@ lzma_stream_buffer_encode(lzma_filter *filters, lzma_check check,
                        || out_pos_ptr == NULL || *out_pos_ptr > out_size)
                return LZMA_PROG_ERROR;
 
+       if (!lzma_check_is_supported(check))
+               return LZMA_UNSUPPORTED_CHECK;
+
        // Note for the paranoids: Index encoder prevents the Stream from
        // getting too big and still being accepted with LZMA_OK, and Block
        // encoder catches if the input is too big. So we don't need to