[dovecot-cvs] dovecot: Moved duplicated log record validation code to a single...
dovecot at dovecot.org
dovecot at dovecot.org
Wed Jun 13 21:07:39 EEST 2007
details: http://hg.dovecot.org/dovecot/rev/8b481177965a
changeset: 5718:8b481177965a
user: Timo Sirainen <tss at iki.fi>
date: Wed Jun 13 21:07:35 2007 +0300
description:
Moved duplicated log record validation code to a single place.
diffstat:
4 files changed, 120 insertions(+), 89 deletions(-)
src/lib-index/mail-index-sync-keywords.c | 19 ---
src/lib-index/mail-index-sync-update.c | 26 +----
src/lib-index/mail-index-sync.c | 20 ----
src/lib-index/mail-transaction-log-view.c | 144 ++++++++++++++++++++++-------
diffs (truncated from 315 to 300 lines):
diff -r 15199d6c9354 -r 8b481177965a src/lib-index/mail-index-sync-keywords.c
--- a/src/lib-index/mail-index-sync-keywords.c Wed Jun 13 20:11:03 2007 +0300
+++ b/src/lib-index/mail-index-sync-keywords.c Wed Jun 13 21:07:35 2007 +0300
@@ -261,21 +261,10 @@ int mail_index_sync_keywords(struct mail
seqset_offset = sizeof(*rec) + rec->name_size;
if ((seqset_offset % 4) != 0)
seqset_offset += 4 - (seqset_offset % 4);
-
- if (seqset_offset > hdr->size) {
- mail_index_sync_set_corrupted(ctx,
- "Keyword header ended unexpectedly");
- return -1;
- }
+ i_assert(seqset_offset < hdr->size);
uid = CONST_PTR_OFFSET(rec, seqset_offset);
end = CONST_PTR_OFFSET(rec, hdr->size);
-
- if (uid == end) {
- mail_index_sync_set_corrupted(ctx,
- "Keyword sequence list empty");
- return -1;
- }
keyword_name = t_strndup(rec + 1, rec->name_size);
if (keyword_lookup(ctx, keyword_name, &keyword_idx) < 0)
@@ -307,12 +296,6 @@ int mail_index_sync_keywords(struct mail
}
while (uid+2 <= end) {
- if (uid[0] > uid[1] || uid[0] == 0) {
- mail_index_sync_set_corrupted(ctx,
- "Keyword record UIDs are broken");
- return -1;
- }
-
ret = keywords_update_records(ctx, ext, keyword_idx,
rec->modify_type,
uid[0], uid[1]);
diff -r 15199d6c9354 -r 8b481177965a src/lib-index/mail-index-sync-update.c
--- a/src/lib-index/mail-index-sync-update.c Wed Jun 13 20:11:03 2007 +0300
+++ b/src/lib-index/mail-index-sync-update.c Wed Jun 13 21:07:35 2007 +0300
@@ -298,18 +298,6 @@ sync_expunge(const struct mail_transacti
map = mail_index_sync_get_atomic_map(ctx);
for (i = 0; i < count; i++, e++) {
- if (e->uid1 > e->uid2 || e->uid1 == 0) {
- mail_index_sync_set_corrupted(ctx,
- "Invalid UID range in expunge (%u .. %u)",
- e->uid1, e->uid2);
- return -1;
- }
- if (i > 0 && e->uid1 <= e[-1].uid2) {
- mail_index_sync_set_corrupted(ctx,
- "Non-sorted UID ranges in expunge");
- return -1;
- }
-
if (mail_index_lookup_uid_range(ctx->view, e->uid1, e->uid2,
&seq1, &seq2) < 0)
return -1;
@@ -416,13 +404,6 @@ static int sync_flag_update(const struct
struct mail_index_record *rec;
uint8_t flag_mask, old_flags;
uint32_t idx, seq1, seq2;
-
- if (u->uid1 > u->uid2 || u->uid1 == 0) {
- mail_index_sync_set_corrupted(ctx,
- "Invalid UID range in flag update (%u .. %u)",
- u->uid1, u->uid2);
- return -1;
- }
if (mail_index_lookup_uid_range(view, u->uid1, u->uid2,
&seq1, &seq2) < 0)
@@ -610,6 +591,13 @@ int mail_index_sync_record(struct mail_i
}
rec = CONST_PTR_OFFSET(data, i);
+ if (i + sizeof(*rec) + rec->name_size > hdr->size) {
+ mail_index_sync_set_corrupted(ctx,
+ "extension intro: name_size too large");
+ ret = -1;
+ break;
+ }
+
ret = mail_index_sync_ext_intro(ctx, rec);
if (ret <= 0)
break;
diff -r 15199d6c9354 -r 8b481177965a src/lib-index/mail-index-sync.c
--- a/src/lib-index/mail-index-sync.c Wed Jun 13 20:11:03 2007 +0300
+++ b/src/lib-index/mail-index-sync.c Wed Jun 13 21:07:35 2007 +0300
@@ -34,18 +34,6 @@ struct mail_index_sync_ctx {
unsigned int sync_dirty:1;
};
-static bool mail_index_sync_check_uid_range(struct mail_index_sync_ctx *ctx,
- uint32_t uid1, uint32_t uid2)
-{
- if (uid1 > uid2 || uid1 == 0) {
- mail_transaction_log_view_set_corrupted(ctx->view->log_view,
- "Broken UID range: %u..%u (type=0x%x)", uid1, uid2,
- ctx->hdr->type & MAIL_TRANSACTION_TYPE_MASK);
- return FALSE;
- }
- return TRUE;
-}
-
static void mail_index_sync_add_expunge(struct mail_index_sync_ctx *ctx)
{
const struct mail_transaction_expunge *e = ctx->data;
@@ -53,8 +41,6 @@ static void mail_index_sync_add_expunge(
uint32_t uid;
for (i = 0; i < size; i++) {
- if (!mail_index_sync_check_uid_range(ctx, e[i].uid1, e[i].uid2))
- break;
for (uid = e[i].uid1; uid <= e[i].uid2; uid++)
mail_index_expunge(ctx->sync_trans, uid);
}
@@ -66,8 +52,6 @@ static void mail_index_sync_add_flag_upd
size_t i, size = ctx->hdr->size / sizeof(*u);
for (i = 0; i < size; i++) {
- if (!mail_index_sync_check_uid_range(ctx, u[i].uid1, u[i].uid2))
- break;
if (u[i].add_flags != 0) {
mail_index_update_flags_range(ctx->sync_trans,
u[i].uid1, u[i].uid2,
@@ -105,8 +89,6 @@ static void mail_index_sync_add_keyword_
size = (ctx->hdr->size - uidset_offset) / sizeof(uint32_t);
for (i = 0; i < size; i += 2) {
/* FIXME: mail_index_update_keywords_range() */
- if (!mail_index_sync_check_uid_range(ctx, uids[i], uids[i+1]))
- break;
for (uid = uids[i]; uid <= uids[i+1]; uid++) {
mail_index_update_keywords(ctx->sync_trans, uid,
u->modify_type, keywords);
@@ -126,8 +108,6 @@ static void mail_index_sync_add_keyword_
keywords = mail_index_keywords_create(ctx->sync_trans, NULL);
for (i = 0; i < size; i++) {
- if (!mail_index_sync_check_uid_range(ctx, u[i].uid1, u[i].uid2))
- break;
for (uid = u[i].uid1; uid <= u[i].uid2; uid++) {
mail_index_update_keywords(ctx->sync_trans, uid,
MODIFY_REPLACE, keywords);
diff -r 15199d6c9354 -r 8b481177965a src/lib-index/mail-transaction-log-view.c
--- a/src/lib-index/mail-transaction-log-view.c Wed Jun 13 20:11:03 2007 +0300
+++ b/src/lib-index/mail-transaction-log-view.c Wed Jun 13 21:07:35 2007 +0300
@@ -320,6 +320,115 @@ mail_transaction_log_view_is_corrupted(s
return view->broken;
}
+static bool
+log_view_is_record_valid(struct mail_transaction_log_file *file,
+ const struct mail_transaction_header *hdr,
+ const void *data,
+ const struct mail_transaction_type_map *type_rec)
+{
+ enum mail_transaction_type hdr_type;
+ ARRAY_TYPE(seq_range) uids = ARRAY_INIT;
+ buffer_t *uid_buf;
+ uint32_t hdr_size;
+ bool ret = TRUE;
+
+ hdr_type = hdr->type & MAIL_TRANSACTION_TYPE_MASK;
+ hdr_size = mail_index_offset_to_uint32(hdr->size) - sizeof(*hdr);
+
+ /* we want to be extra careful with expunges */
+ if ((hdr->type & MAIL_TRANSACTION_EXPUNGE) != 0) {
+ if (hdr_type != (MAIL_TRANSACTION_EXPUNGE |
+ MAIL_TRANSACTION_EXPUNGE_PROT)) {
+ mail_transaction_log_file_set_corrupted(file,
+ "expunge record missing protection mask");
+ return FALSE;
+ }
+ } else if (hdr_type != type_rec->type) {
+ mail_transaction_log_file_set_corrupted(file,
+ "extra bits in header type: 0x%x", hdr_type);
+ return FALSE;
+ }
+
+ /* records that are exported by syncing and view syncing will be
+ checked here so that we don't have to implement the same validation
+ multiple times. other records are checked internally by
+ mail_index_sync_record(). */
+ t_push();
+ switch (hdr_type) {
+ case MAIL_TRANSACTION_EXPUNGE: {
+ uid_buf = buffer_create_const_data(pool_datastack_create(),
+ data, hdr_size);
+ array_create_from_buffer(&uids, uid_buf,
+ sizeof(struct mail_transaction_expunge));
+ break;
+ }
+ case MAIL_TRANSACTION_FLAG_UPDATE: {
+ uid_buf = buffer_create_const_data(pool_datastack_create(),
+ data, hdr_size);
+ array_create_from_buffer(&uids, uid_buf,
+ sizeof(struct mail_transaction_flag_update));
+ break;
+ }
+ case MAIL_TRANSACTION_KEYWORD_UPDATE: {
+ const struct mail_transaction_keyword_update *rec = data;
+ unsigned int seqset_offset;
+
+ seqset_offset = sizeof(*rec) + rec->name_size;
+ if ((seqset_offset % 4) != 0)
+ seqset_offset += 4 - (seqset_offset % 4);
+
+ if (seqset_offset >= hdr_size ||
+ ((hdr_size - seqset_offset) % (sizeof(uint32_t)*2)) != 0) {
+ mail_transaction_log_file_set_corrupted(file,
+ "Invalid keyword update record size");
+ ret = FALSE;
+ break;
+ }
+
+ uid_buf = buffer_create_const_data(pool_datastack_create(),
+ CONST_PTR_OFFSET(data, seqset_offset),
+ hdr_size - seqset_offset);
+ array_create_from_buffer(&uids, uid_buf, sizeof(uint32_t)*2);
+ break;
+ }
+ case MAIL_TRANSACTION_KEYWORD_RESET: {
+ uid_buf = buffer_create_const_data(pool_datastack_create(),
+ data, hdr_size);
+ array_create_from_buffer(&uids, uid_buf,
+ sizeof(struct mail_transaction_keyword_reset));
+ break;
+ }
+ default:
+ break;
+ }
+
+ if (array_is_created(&uids)) {
+ const struct seq_range *rec, *prev = NULL;
+ unsigned int i, count = array_count(&uids);
+
+ for (i = 0; i < count; i++, prev = rec) {
+ rec = array_idx(&uids, i);
+ if (rec->seq1 > rec->seq2 || rec->seq1 == 0) {
+ mail_transaction_log_file_set_corrupted(file,
+ "Invalid UID range "
+ "(%u .. %u, type=0x%x)",
+ rec->seq1, rec->seq2, hdr_type);
+ ret = FALSE;
+ break;
+ }
+ if (prev != NULL && rec->seq1 <= prev->seq2) {
+ mail_transaction_log_file_set_corrupted(file,
+ "Non-sorted UID ranges (type=0x%x)",
+ hdr_type);
+ ret = FALSE;
+ break;
+ }
+ }
+ }
+ t_pop();
+ return ret;
+}
+
static int
log_view_get_next(struct mail_transaction_log_view *view,
const struct mail_transaction_header **hdr_r,
@@ -373,6 +482,7 @@ log_view_get_next(struct mail_transactio
hdr_type = hdr->type & MAIL_TRANSACTION_TYPE_MASK;
hdr_size = mail_index_offset_to_uint32(hdr->size);
if (hdr_size < sizeof(*hdr)) {
+ /* we'll fail below with "records size too small" */
type_rec = NULL;
record_size = 0;
} else {
@@ -412,38 +522,8 @@ log_view_get_next(struct mail_transactio
return -1;
}
- if ((hdr->type & MAIL_TRANSACTION_EXPUNGE) != 0) {
- if (hdr_type != (MAIL_TRANSACTION_EXPUNGE |
- MAIL_TRANSACTION_EXPUNGE_PROT)) {
- mail_transaction_log_file_set_corrupted(file,
- "found expunge without protection mask");
- return -1;
- }
- } else if (hdr_type != type_rec->type) {
- mail_transaction_log_file_set_corrupted(file,
- "extra bits in header type: 0x%x", hdr_type);
- return -1;
- } else if (hdr_type == MAIL_TRANSACTION_EXT_INTRO) {
- const struct mail_transaction_ext_intro *intro;
- uint32_t i;
-
- for (i = 0; i < hdr_size; ) {
- if (i + sizeof(*intro) > hdr_size) {
- /* should be just extra padding */
- break;
- }
-
- intro = CONST_PTR_OFFSET(data, i);
More information about the dovecot-cvs
mailing list