Mercurial > dovecot > original-hg > dovecot-1.2
changeset 6353:23c9ac999578 HEAD
mailbox_keywords_create() checks keyword validity now and returns failure if
any of them are invalid. Added mailbox_keywords_create_valid() that only
drops invalid keywords. Removed mbox and IMAP-specific keyword checks, they
all use the same checking now.
| author | Timo Sirainen <tss@iki.fi> |
|---|---|
| date | Sun, 02 Sep 2007 06:10:54 +0300 |
| parents | 0f1a4b7b39a3 |
| children | 8476d665530f |
| files | src/deliver/deliver.c src/imap/cmd-append.c src/imap/cmd-copy.c src/imap/cmd-store.c src/imap/commands-util.c src/imap/common.h src/imap/main.c src/lib-storage/index/index-storage.c src/lib-storage/index/index-storage.h src/lib-storage/index/mbox/mbox-sync-parse.c src/lib-storage/mail-storage-private.h src/lib-storage/mail-storage.c src/lib-storage/mail-storage.h src/plugins/convert/convert-storage.c |
| diffstat | 14 files changed, 156 insertions(+), 96 deletions(-) [+] |
line wrap: on
line diff
--- a/src/deliver/deliver.c Sun Sep 02 06:08:58 2007 +0300 +++ b/src/deliver/deliver.c Sun Sep 02 06:10:54 2007 +0300 @@ -142,7 +142,7 @@ t = mailbox_transaction_begin(box, MAILBOX_TRANSACTION_FLAG_EXTERNAL); kw = strarray_length(keywords) == 0 ? NULL : - mailbox_keywords_create(t, keywords); + mailbox_keywords_create_valid(t, keywords); if (mailbox_copy(t, mail, flags, kw, NULL) < 0) ret = -1; mailbox_keywords_free(t, &kw);
--- a/src/imap/cmd-append.c Sun Sep 02 06:08:58 2007 +0300 +++ b/src/imap/cmd-append.c Sun Sep 02 06:10:54 2007 +0300 @@ -290,8 +290,13 @@ if (!client_parse_mail_flags(cmd, flags_list, &flags, &keywords_list)) return cmd_append_cancel(ctx, nonsync); - keywords = keywords_list == NULL ? NULL : - mailbox_keywords_create(ctx->t, keywords_list); + if (keywords_list == NULL) + keywords = NULL; + else if (mailbox_keywords_create(ctx->t, keywords_list, + &keywords) < 0) { + client_send_storage_error(cmd, ctx->storage); + return cmd_append_cancel(ctx, nonsync); + } } else { flags = 0; keywords = NULL;
--- a/src/imap/cmd-copy.c Sun Sep 02 06:08:58 2007 +0300 +++ b/src/imap/cmd-copy.c Sun Sep 02 06:10:54 2007 +0300 @@ -62,7 +62,7 @@ keywords_list = mail_get_keywords(mail); keywords = strarray_length(keywords_list) == 0 ? NULL : - mailbox_keywords_create(t, keywords_list); + mailbox_keywords_create_valid(t, keywords_list); if (mailbox_copy(t, mail, mail_get_flags(mail), keywords, NULL) < 0) ret = mail->expunged ? 0 : -1;
--- a/src/imap/cmd-store.c Sun Sep 02 06:08:58 2007 +0300 +++ b/src/imap/cmd-store.c Sun Sep 02 06:10:54 2007 +0300 @@ -88,8 +88,14 @@ t = mailbox_transaction_begin(box, !silent ? 0 : MAILBOX_TRANSACTION_FLAG_HIDE); - keywords = keywords_list != NULL || modify_type == MODIFY_REPLACE ? - mailbox_keywords_create(t, keywords_list) : NULL; + if (keywords_list == NULL && modify_type != MODIFY_REPLACE) + keywords = NULL; + else if (mailbox_keywords_create(t, keywords_list, &keywords) < 0) { + /* invalid keywords */ + mailbox_transaction_rollback(&t); + client_send_storage_error(cmd, mailbox_get_storage(box)); + return TRUE; + } search_ctx = mailbox_search_init(t, NULL, search_arg, NULL); mail = mail_alloc(t, MAIL_FETCH_FLAGS, NULL);
--- a/src/imap/commands-util.c Sun Sep 02 06:08:58 2007 +0300 +++ b/src/imap/commands-util.c Sun Sep 02 06:10:54 2007 +0300 @@ -189,45 +189,17 @@ client_send_line(client, t_strconcat("* NO ", error_string, NULL)); } -static bool is_valid_keyword(struct client_command_context *cmd, - const char *keyword) -{ - const char *const *names; - unsigned int i, count; - - /* if it already exists, skip validity checks */ - if (array_is_created(&cmd->client->keywords.keywords)) { - names = array_get(&cmd->client->keywords.keywords, &count); - for (i = 0; i < count; i++) { - if (strcasecmp(names[i], keyword) == 0) - return TRUE; - } - } - - if (strlen(keyword) > max_keyword_length) { - client_send_tagline(cmd, - t_strdup_printf("BAD Invalid keyword name '%s': " - "Maximum length is %u characters", - keyword, max_keyword_length)); - return FALSE; - } - - return TRUE; -} - bool client_parse_mail_flags(struct client_command_context *cmd, const struct imap_arg *args, enum mail_flags *flags_r, const char *const **keywords_r) { - const char *const *keywords; const char *atom; - buffer_t *buffer; - size_t size, i; + ARRAY_DEFINE(keywords, const char *); *flags_r = 0; *keywords_r = NULL; - buffer = buffer_create_dynamic(cmd->pool, 256); + p_array_init(&keywords, cmd->pool, 16); while (args->type != IMAP_ARG_EOL) { if (args->type != IMAP_ARG_ATOM) { @@ -257,28 +229,19 @@ return FALSE; } } else { - /* keyword - first make sure it's not a duplicate */ - keywords = buffer_get_data(buffer, &size); - size /= sizeof(const char *); - for (i = 0; i < size; i++) { - if (strcasecmp(keywords[i], atom) == 0) - break; - } - - if (i == size) { - if (!is_valid_keyword(cmd, atom)) - return FALSE; - buffer_append(buffer, &atom, sizeof(atom)); - } + /* keyword validity checks are done by lib-storage */ + array_append(&keywords, &atom, 1); } args++; } - atom = NULL; - buffer_append(buffer, &atom, sizeof(atom)); - *keywords_r = buffer->used == sizeof(atom) ? NULL : - buffer_get_data(buffer, NULL); + if (array_count(&keywords) == 0) + *keywords_r = NULL; + else { + (void)array_append_space(&keywords); /* NULL-terminate */ + *keywords_r = array_idx(&keywords, 0); + } return TRUE; }
--- a/src/imap/common.h Sun Sep 02 06:08:58 2007 +0300 +++ b/src/imap/common.h Sun Sep 02 06:10:54 2007 +0300 @@ -21,8 +21,6 @@ by default. */ #define DEFAULT_IMAP_MAX_LINE_LENGTH 65536 -#define DEFAULT_MAX_KEYWORD_LENGTH 50 - enum client_workarounds { WORKAROUND_DELAY_NEWMAIL = 0x01, WORKAROUND_NETSCAPE_EOH = 0x04, @@ -30,7 +28,6 @@ }; extern struct ioloop *ioloop; -extern unsigned int max_keyword_length; extern unsigned int imap_max_line_length; extern enum client_workarounds client_workarounds; extern const char *logout_format;
--- a/src/imap/main.c Sun Sep 02 06:08:58 2007 +0300 +++ b/src/imap/main.c Sun Sep 02 06:10:54 2007 +0300 @@ -39,7 +39,6 @@ }; struct ioloop *ioloop; -unsigned int max_keyword_length; unsigned int imap_max_line_length; enum client_workarounds client_workarounds = 0; const char *logout_format; @@ -225,11 +224,6 @@ (unsigned int)strtoul(str, NULL, 10) : DEFAULT_IMAP_MAX_LINE_LENGTH; - str = getenv("MAIL_MAX_KEYWORD_LENGTH"); - max_keyword_length = str != NULL ? - (unsigned int)strtoul(str, NULL, 10) : - DEFAULT_MAX_KEYWORD_LENGTH; - logout_format = getenv("IMAP_LOGOUT_FORMAT"); if (logout_format == NULL) logout_format = "bytes=%i/%o";
--- a/src/lib-storage/index/index-storage.c Sun Sep 02 06:08:58 2007 +0300 +++ b/src/lib-storage/index/index-storage.c Sun Sep 02 06:10:54 2007 +0300 @@ -4,6 +4,7 @@ #include "array.h" #include "buffer.h" #include "ioloop.h" +#include "imap-parser.h" #include "mkdir-parents.h" #include "mail-index-private.h" #include "index-storage.h" @@ -469,14 +470,86 @@ mail_index_reset_error(ibox->index); } -struct mail_keywords * -index_keywords_create(struct mailbox_transaction_context *_t, - const char *const keywords[]) +int index_mailbox_keyword_is_valid(struct index_mailbox *ibox, + const char *keyword, const char **error_r) +{ + unsigned int i, idx; + + /* if it already exists, skip validity checks */ + if (mail_index_keyword_lookup(ibox->index, keyword, &idx)) + return TRUE; + + if (*keyword == '\0') { + *error_r = "Empty keywords not allowed"; + return FALSE; + } + + /* these are IMAP-specific restrictions, but for now IMAP is all we + care about */ + for (i = 0; keyword[i] != '\0'; i++) { + if (IS_ATOM_SPECIAL((unsigned char)keyword[i])) { + *error_r = "Invalid characters in keyword"; + return FALSE; + } + if ((unsigned char)keyword[i] >= 0x80) { + *error_r = "8bit characters in keyword"; + return FALSE; + } + } + if (i > ibox->box.storage->keyword_max_len) { + *error_r = "Keyword length too long"; + return FALSE; + } + return TRUE; +} + +static struct mail_keywords * +index_keywords_create_skip(struct index_transaction_context *t, + const char *const keywords[]) +{ + ARRAY_DEFINE(valid_keywords, const char *); + struct mail_keywords *kw; + const char *error; + + t_push(); + t_array_init(&valid_keywords, 32); + for (; *keywords != NULL; keywords++) { + if (index_mailbox_keyword_is_valid(t->ibox, *keywords, &error)) + array_append(&valid_keywords, keywords, 1); + } + (void)array_append_space(&valid_keywords); /* NULL-terminate */ + kw = mail_index_keywords_create(t->trans, keywords); + t_pop(); + return kw; +} + +int index_keywords_create(struct mailbox_transaction_context *_t, + const char *const keywords[], + struct mail_keywords **keywords_r, bool skip_invalid) { struct index_transaction_context *t = (struct index_transaction_context *)_t; + const char *error; + unsigned int i; - return mail_index_keywords_create(t->trans, keywords); + for (i = 0; keywords[i] != NULL; i++) { + if (!index_mailbox_keyword_is_valid(t->ibox, keywords[i], + &error)) { + if (skip_invalid) { + /* found invalid keywords, do this the slow + way */ + *keywords_r = + index_keywords_create_skip(t, keywords); + return 0; + } + mail_storage_set_error(t->ibox->box.storage, + MAIL_ERROR_PARAMS, error); + return -1; + } + } + + *keywords_r = mail_index_keywords_create(t->trans, keywords); + return 0; } void index_keywords_free(struct mailbox_transaction_context *t __attr_unused__,
--- a/src/lib-storage/index/index-storage.h Sun Sep 02 06:08:58 2007 +0300 +++ b/src/lib-storage/index/index-storage.h Sun Sep 02 06:10:54 2007 +0300 @@ -109,9 +109,11 @@ bool index_storage_allow_new_keywords(struct mailbox *box); bool index_storage_is_inconsistent(struct mailbox *box); -struct mail_keywords * -index_keywords_create(struct mailbox_transaction_context *t, - const char *const keywords[]); +int index_mailbox_keyword_is_valid(struct index_mailbox *ibox, + const char *keyword, const char **error_r); +int index_keywords_create(struct mailbox_transaction_context *t, + const char *const keywords[], + struct mail_keywords **keywords_r, bool skip_invalid); void index_keywords_free(struct mailbox_transaction_context *t, struct mail_keywords *keywords);
--- a/src/lib-storage/index/mbox/mbox-sync-parse.c Sun Sep 02 06:08:58 2007 +0300 +++ b/src/lib-storage/index/mbox/mbox-sync-parse.c Sun Sep 02 06:10:54 2007 +0300 @@ -109,24 +109,11 @@ return TRUE; } -static bool keyword_is_valid(const char *keyword) -{ - /* try to only prevent the most malicious looking keywords. */ - for (; *keyword != '\0'; keyword++) { - if (*keyword == '(' || *keyword == ')' || - *keyword == '{' || *keyword == '}' || - *keyword == '\\' || *keyword == '"' || - (unsigned char)*keyword <= 32) - return FALSE; - } - return TRUE; -} - static void parse_imap_keywords_list(struct mbox_sync_mail_context *ctx, struct message_header_line *hdr, size_t pos) { - const char *keyword; + const char *keyword, *error; size_t keyword_start; unsigned int idx, count; @@ -148,7 +135,8 @@ t_push(); keyword = t_strndup(hdr->full_value + keyword_start, pos - keyword_start); - if (keyword_is_valid(keyword)) { + if (index_mailbox_keyword_is_valid(&ctx->sync_ctx->mbox->ibox, + keyword, &error)) { mail_index_keyword_lookup_or_create( ctx->sync_ctx->mbox->ibox.index, keyword, &idx); }
--- a/src/lib-storage/mail-storage-private.h Sun Sep 02 06:08:58 2007 +0300 +++ b/src/lib-storage/mail-storage-private.h Sun Sep 02 06:10:54 2007 +0300 @@ -61,7 +61,8 @@ const char *user; /* name of user accessing the storage */ enum mail_storage_flags flags; - enum file_lock_method lock_method; + enum file_lock_method lock_method; + unsigned int keyword_max_len; struct mail_storage_callbacks *callbacks; void *callback_context; @@ -116,9 +117,10 @@ uint32_t *last_saved_uid_r); void (*transaction_rollback)(struct mailbox_transaction_context *t); - struct mail_keywords * - (*keywords_create)(struct mailbox_transaction_context *t, - const char *const keywords[]); + int (*keywords_create)(struct mailbox_transaction_context *t, + const char *const keywords[], + struct mail_keywords **keywords_r, + bool skip_invalid); void (*keywords_free)(struct mailbox_transaction_context *t, struct mail_keywords *keywords);
--- a/src/lib-storage/mail-storage.c Sun Sep 02 06:08:58 2007 +0300 +++ b/src/lib-storage/mail-storage.c Sun Sep 02 06:10:54 2007 +0300 @@ -19,6 +19,8 @@ "Internal error occurred. Refer to server log for more information." #define CRITICAL_MSG_STAMP CRITICAL_MSG " [%Y-%m-%d %H:%M:%S]" +#define DEFAULT_MAX_KEYWORD_LENGTH 50 + struct mail_storage_module_register mail_storage_module_register = { 0 }; struct mail_module_register mail_module_register = { 0 }; @@ -169,7 +171,7 @@ { struct mail_storage *storage_class, *storage; struct mail_storage *const *classes; - const char *home; + const char *home, *value; unsigned int i, count; if (data == NULL) @@ -240,6 +242,10 @@ return -1; } + value = getenv("MAIL_MAX_KEYWORD_LENGTH"); + storage->keyword_max_len = value != NULL ? + atoi(value) : DEFAULT_MAX_KEYWORD_LENGTH; + if (hook_mail_storage_created != NULL) hook_mail_storage_created(storage); @@ -531,11 +537,29 @@ mailbox_notify_changes(box, 0, NULL, NULL); } +int mailbox_keywords_create(struct mailbox_transaction_context *t, + const char *const keywords[], + struct mail_keywords **keywords_r) +{ + const char *empty_keyword_list = NULL; + + if (keywords == NULL) + keywords = &empty_keyword_list; + return t->box->v.keywords_create(t, keywords, keywords_r, FALSE); +} + struct mail_keywords * -mailbox_keywords_create(struct mailbox_transaction_context *t, - const char *const keywords[]) +mailbox_keywords_create_valid(struct mailbox_transaction_context *t, + const char *const keywords[]) { - return t->box->v.keywords_create(t, keywords); + const char *empty_keyword_list = NULL; + struct mail_keywords *kw; + + if (keywords == NULL) + keywords = &empty_keyword_list; + if (t->box->v.keywords_create(t, keywords, &kw, TRUE) < 0) + i_unreached(); + return kw; } void mailbox_keywords_free(struct mailbox_transaction_context *t,
--- a/src/lib-storage/mail-storage.h Sun Sep 02 06:08:58 2007 +0300 +++ b/src/lib-storage/mail-storage.h Sun Sep 02 06:10:54 2007 +0300 @@ -322,10 +322,15 @@ /* Return the number of active transactions for the mailbox. */ unsigned int mailbox_transaction_get_count(struct mailbox *box); -/* Build mail_keywords from NULL-terminated keywords list. */ +/* Build mail_keywords from NULL-terminated keywords list. + Returns 0 if successful, -1 if there are invalid keywords (error is set). */ +int mailbox_keywords_create(struct mailbox_transaction_context *t, + const char *const keywords[], + struct mail_keywords **keywords_r); +/* Like mailbox_keywords_create(), except ignore invalid keywords. */ struct mail_keywords * -mailbox_keywords_create(struct mailbox_transaction_context *t, - const char *const keywords[]); +mailbox_keywords_create_valid(struct mailbox_transaction_context *t, + const char *const keywords[]); void mailbox_keywords_free(struct mailbox_transaction_context *t, struct mail_keywords **keywords);
--- a/src/plugins/convert/convert-storage.c Sun Sep 02 06:08:58 2007 +0300 +++ b/src/plugins/convert/convert-storage.c Sun Sep 02 06:10:54 2007 +0300 @@ -72,7 +72,8 @@ keywords_list = mail_get_keywords(mail); keywords = strarray_length(keywords_list) == 0 ? NULL : - mailbox_keywords_create(dest_trans, keywords_list); + mailbox_keywords_create_valid(dest_trans, + keywords_list); ret = mailbox_copy(dest_trans, mail, mail_get_flags(mail), keywords, NULL);