lib-index: mail_index_sync_ext_atomic_inc() does now better error checking.

--HG--
branch : HEAD

diff --git a/src/lib-index/Makefile.am b/src/lib-index/Makefile.am
index e3e8085e047dedc154621689f03c3e8eb504fc96..34c80ceaa68ceba3358de558a8fe4d920deb34b8 100644 (file)
--- a/src/lib-index/Makefile.am
+++ b/src/lib-index/Makefile.am
@@ -65,6 +65,7 @@ headers = \
         mailbox-log.h
 
 test_programs = \
+       test-mail-index-sync-ext \
        test-mail-index-transaction-finish \
        test-mail-index-transaction-update \
        test-mail-transaction-log-append \
@@ -77,6 +78,10 @@ test_libs = \
        ../lib-test/libtest.la \
        ../lib/liblib.la
 
+test_mail_index_sync_ext_SOURCES = test-mail-index-sync-ext.c
+test_mail_index_sync_ext_LDADD = mail-index-sync-ext.lo $(test_libs)
+test_mail_index_sync_ext_DEPENDENCIES = mail-index-sync-ext.lo $(test_libs)
+
 test_mail_index_transaction_finish_SOURCES = test-mail-index-transaction-finish.c
 test_mail_index_transaction_finish_LDADD = mail-index-transaction-finish.lo $(test_libs)
 test_mail_index_transaction_finish_DEPENDENCIES = mail-index-transaction-finish.lo $(test_libs)

diff --git a/src/lib-index/mail-index-sync-ext.c b/src/lib-index/mail-index-sync-ext.c
index 6ce046e18a08a4410ae1e76a3dc8a0d3f8f2b0d2..ba29c710de8457ebf7c006014ccd0cdd48bb1211 100644 (file)
--- a/src/lib-index/mail-index-sync-ext.c
+++ b/src/lib-index/mail-index-sync-ext.c
@@ -699,7 +699,7 @@ mail_index_sync_ext_atomic_inc(struct mail_index_sync_map_ctx *ctx,
        const struct mail_index_ext *ext;
        void *data;
        uint32_t seq;
-       uint64_t orig_num;
+       uint64_t min_value, max_value, orig_num;
 
        i_assert(ctx->cur_ext_map_idx != (uint32_t)-1);
        i_assert(!ctx->cur_ext_ignore);
@@ -720,30 +720,49 @@ mail_index_sync_ext_atomic_inc(struct mail_index_sync_map_ctx *ctx,
        rec = MAIL_INDEX_MAP_IDX(view->map, seq-1);
        data = PTR_OFFSET(rec, ext->record_offset);
 
+       min_value = u->diff >= 0 ? 0 : (uint64_t)(-(int64_t)u->diff);
+
+       max_value = ext->record_size == 8 ? (uint64_t)-1 :
+               ((uint64_t)1 << (ext->record_size*8)) - 1;
+       if (u->diff <= 0) {
+               /* skip */
+       } else if (max_value >= (uint32_t)u->diff) {
+               max_value -= u->diff;
+       } else {
+               mail_index_sync_set_corrupted(ctx,
+                       "Extension record inc diff=%d larger than max value=%u "
+                       "(uid=%u)", u->diff, (unsigned int)max_value, u->uid);
+               return -1;
+       }
+
        switch (ext->record_size) {
        case 1: {
                uint8_t *num = data;
 
                orig_num = *num;
-               *num += u->diff;
+               if (orig_num >= min_value && orig_num <= max_value)
+                       *num += u->diff;
                break;
        }
        case 2: {
                uint16_t *num = data;
                orig_num = *num;
-               *num += u->diff;
+               if (orig_num >= min_value && orig_num <= max_value)
+                       *num += u->diff;
                break;
        }
        case 4: {
                uint32_t *num = data;
                orig_num = *num;
-               *num += u->diff;
+               if (orig_num >= min_value && orig_num <= max_value)
+                       *num += u->diff;
                break;
        }
        case 8: {
                uint64_t *num = data;
                orig_num = *num;
-               *num += u->diff;
+               if (orig_num >= min_value && orig_num <= max_value)
+                       *num += u->diff;
                break;
        }
        default:
@@ -752,12 +771,18 @@ mail_index_sync_ext_atomic_inc(struct mail_index_sync_map_ctx *ctx,
                        ext->record_size);
                return -1;
        }
-       if (u->diff < 0 && (uint64_t)(-(int64_t)u->diff) > orig_num) {
+       if (orig_num < min_value) {
                mail_index_sync_set_corrupted(ctx,
                        "Extension record inc drops number below zero "
                        "(uid=%u, diff=%d, orig=%llu)",
                        u->uid, u->diff, (unsigned long long)orig_num);
                return -1;
+       } else if (orig_num > max_value) {
+               mail_index_sync_set_corrupted(ctx,
+                       "Extension record inc overflows number "
+                       "(uid=%u, diff=%d, orig=%llu)",
+                       u->uid, u->diff, (unsigned long long)orig_num);
+               return -1;
        }
 
        mail_index_sync_write_seq_update(ctx, seq, seq);

diff --git a/src/lib-index/mail-index-sync-private.h b/src/lib-index/mail-index-sync-private.h
index 147b421b10d532a7e54a97ed396bf8342c0f35f6..8c30052314f799ad6ca68554e4c7fa696f7bf277 100644 (file)
--- a/src/lib-index/mail-index-sync-private.h
+++ b/src/lib-index/mail-index-sync-private.h
@@ -1,6 +1,7 @@
 #ifndef MAIL_INDEX_SYNC_PRIVATE_H
 #define MAIL_INDEX_SYNC_PRIVATE_H
 
+#include "mail-index-private.h"
 #include "mail-transaction-log.h"
 
 struct uid_range {

diff --git a/src/lib-index/test-mail-index-sync-ext.c b/src/lib-index/test-mail-index-sync-ext.c
new file mode 100644 (file)
index 0000000..5bb9ff4
--- /dev/null
+++ b/src/lib-index/test-mail-index-sync-ext.c
@@ -0,0 +1,102 @@
+/* Copyright (c) 2010 Dovecot authors, see the included COPYING file */
+
+#include "lib.h"
+#include "array.h"
+#include "test-common.h"
+#include "mail-index-sync-private.h"
+#include "mail-index-modseq.h"
+
+#include <stdlib.h>
+
+void mail_index_sync_set_corrupted(struct mail_index_sync_map_ctx *ctx ATTR_UNUSED,
+                                  const char *fmt ATTR_UNUSED, ...) {}
+struct mail_index_map *
+mail_index_sync_get_atomic_map(struct mail_index_sync_map_ctx *ctx) { return ctx->view->map; }
+uint32_t
+mail_index_map_register_ext(struct mail_index_map *map ATTR_UNUSED,
+                           const char *name ATTR_UNUSED, uint32_t ext_offset ATTR_UNUSED,
+                           const struct mail_index_ext_header *ext_hdr ATTR_UNUSED) { return 0; }
+bool mail_index_ext_lookup(struct mail_index *index ATTR_UNUSED,
+                          const char *name ATTR_UNUSED,
+                          uint32_t *ext_id_r ATTR_UNUSED) { return FALSE; }
+bool mail_index_map_lookup_ext(struct mail_index_map *map ATTR_UNUSED,
+                              const char *name ATTR_UNUSED,
+                              uint32_t *idx_r ATTR_UNUSED) { return FALSE; }
+int mail_index_map_ext_hdr_check(const struct mail_index_header *hdr ATTR_UNUSED,
+                                const struct mail_index_ext_header *ext_hdr ATTR_UNUSED,
+                                const char *name ATTR_UNUSED,
+                                const char **error_r ATTR_UNUSED) { return -1; }
+void mail_index_modseq_hdr_update(struct mail_index_modseq_sync *ctx ATTR_UNUSED) {}
+bool mail_index_lookup_seq(struct mail_index_view *view ATTR_UNUSED,
+                          uint32_t uid, uint32_t *seq_r) {
+       *seq_r = uid;
+       return TRUE;
+}
+void mail_index_sync_write_seq_update(struct mail_index_sync_map_ctx *ctx ATTR_UNUSED,
+                                     uint32_t seq1 ATTR_UNUSED,
+                                     uint32_t seq2 ATTR_UNUSED) {}
+
+static void test_mail_index_sync_ext_atomic_inc(void)
+{
+       struct mail_index_sync_map_ctx ctx;
+       struct mail_transaction_ext_atomic_inc u;
+       struct mail_index_ext *ext;
+       void *ptr;
+
+       test_begin("mail index sync ext atomic inc");
+
+       memset(&ctx, 0, sizeof(ctx));
+       ctx.view = t_new(struct mail_index_view, 1);
+       ctx.view->map = t_new(struct mail_index_map, 1);
+       ctx.view->map->hdr.next_uid = 2;
+       ctx.view->map->hdr.record_size = sizeof(struct mail_index_record) + 16;
+       ctx.view->map->rec_map = t_new(struct mail_index_record_map, 1);
+       ctx.view->map->rec_map->records =
+               t_malloc(ctx.view->map->hdr.record_size);
+       t_array_init(&ctx.view->map->extensions, 4);
+       ext = array_append_space(&ctx.view->map->extensions);
+       ext->record_offset = sizeof(struct mail_index_record);
+       ptr = PTR_OFFSET(ctx.view->map->rec_map->records, ext->record_offset);
+
+       memset(&u, 0, sizeof(u));
+       test_assert(mail_index_sync_ext_atomic_inc(&ctx, &u) == -1);
+
+       u.uid = 2;
+       test_assert(mail_index_sync_ext_atomic_inc(&ctx, &u) == -1);
+
+       u.uid = 1;
+#define TEST_ATOMIC(_type, _value, _diff, _ret) \
+       { _type *n = ptr; *n = _value; } \
+       ext->record_size = sizeof(_type); \
+       u.diff = _diff; \
+       test_assert(mail_index_sync_ext_atomic_inc(&ctx, &u) == _ret);
+
+#define TEST_ATOMIC_BLOCK(_type, _max) \
+       TEST_ATOMIC(_type, 1, -1, 1); \
+       TEST_ATOMIC(_type, 1, -2, -1); \
+       TEST_ATOMIC(_type, 0, -1, -1); \
+       TEST_ATOMIC(_type, 0, _max, 1); \
+       TEST_ATOMIC(_type, 1, _max, -1); \
+       TEST_ATOMIC(_type, 0, (_max+1), -1); \
+       TEST_ATOMIC(_type, _max, 1, -1); \
+       TEST_ATOMIC(_type, _max, -_max, 1); \
+       TEST_ATOMIC(_type, _max, -(_max+1), -1);
+
+       TEST_ATOMIC_BLOCK(uint8_t, 255);
+       TEST_ATOMIC_BLOCK(uint16_t, 65535);
+
+       ext->record_size = 5;
+       u.diff = 0;
+       test_assert(mail_index_sync_ext_atomic_inc(&ctx, &u) == -1);
+
+       test_end();
+}
+
+int main(void)
+{
+       static void (*test_functions[])(void) = {
+               test_mail_index_sync_ext_atomic_inc,
+               NULL
+       };
+       return test_run(test_functions);
+}