[balsa] imap-handle: check for non-compliant responses
- From: Peter Bloomfield <peterb src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [balsa] imap-handle: check for non-compliant responses
- Date: Sun, 21 Mar 2021 21:13:16 +0000 (UTC)
commit 26e554accc1b8162069716b836b497dd8a71b07d
Author: Peter Bloomfield <PeterBloomfield bellsouth net>
Date: Sun Mar 21 17:10:04 2021 -0400
imap-handle: check for non-compliant responses
* libbalsa/imap/imap-handle.c
(imap_cmd_step): disconnect if a protocol error is found;
(ir_exists): check that the handle is either authenticated or selected;
(ir_recent): ditto;
(ir_fetch_seq): check that the handle is selected;
(ir_expunge): ditto; also check that the sequence number of the expunged message is valid.
ChangeLog | 12 +++++++++++
libbalsa/imap/imap-handle.c | 51 +++++++++++++++++++++++++++++++++++++++++++--
2 files changed, 61 insertions(+), 2 deletions(-)
---
diff --git a/ChangeLog b/ChangeLog
index ea723e8dd..93136f61a 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,15 @@
+2021-03-21 Peter Bloomfield <pbloomfield bellsouth net>
+
+ imap-handle: check for non-compliant responses
+
+ * libbalsa/imap/imap-handle.c
+ (imap_cmd_step): disconnect if a protocol error is found;
+ (ir_exists): check that the handle is either authenticated or selected;
+ (ir_recent): ditto;
+ (ir_fetch_seq): check that the handle is selected;
+ (ir_expunge): ditto; also check that the sequence number of
+ the expunged message is valid.
+
2021-03-09 Peter Bloomfield <pbloomfield bellsouth net>
Try to avoid critical messages
diff --git a/libbalsa/imap/imap-handle.c b/libbalsa/imap/imap-handle.c
index eb045d9a7..1e3b0cab3 100644
--- a/libbalsa/imap/imap-handle.c
+++ b/libbalsa/imap/imap-handle.c
@@ -1809,6 +1809,11 @@ imap_cmd_step(ImapMboxHandle* handle, unsigned lastcmd)
if(rc == IMR_BYE) {
return handle->doing_logout ? IMR_UNTAGGED : IMR_BYE;
}
+ if (rc != IMR_OK) {
+ g_warning("Encountered protocol error while handling untagged response.");
+ imap_handle_disconnect(handle);
+ return IMR_BAD;
+ }
return IMR_UNTAGGED;
}
@@ -2595,7 +2600,14 @@ static ImapResponse
ir_exists(ImapMboxHandle *h, unsigned seqno)
{
unsigned old_exists = h->exists;
- ImapResponse rc = ir_check_crlf(h, sio_getc(h->sio));
+ ImapResponse rc;
+ if ((h->state != IMHS_AUTHENTICATED) && (h->state != IMHS_SELECTED)) {
+ /* bad state for a response to SELECT or EXAMINE, see RFC 3501, Sect. 6.4. and 7.3.1. */
+ g_info("received EXISTS response in bad state %d", h->state);
+ return IMR_PROTOCOL;
+ }
+
+ rc = ir_check_crlf(h, sio_getc(h->sio));
imap_mbox_resize_cache(h, seqno);
mbox_view_resize(&h->mbox_view, old_exists, seqno);
@@ -2607,6 +2619,12 @@ ir_exists(ImapMboxHandle *h, unsigned seqno)
static ImapResponse
ir_recent(ImapMboxHandle *h, unsigned seqno)
{
+ if ((h->state != IMHS_AUTHENTICATED) && (h->state != IMHS_SELECTED)) {
+ /* bad state for a response to SELECT or EXAMINE, see RFC 3501, Sect. 6.4. and 7.3.2. */
+ g_info("received RECENT response in bad state %d", h->state);
+ return IMR_PROTOCOL;
+ }
+
h->recent = seqno;
/* FIXME: send a signal here! */
return ir_check_crlf(h, sio_getc(h->sio));
@@ -2615,11 +2633,34 @@ ir_recent(ImapMboxHandle *h, unsigned seqno)
static ImapResponse
ir_expunge(ImapMboxHandle *h, unsigned seqno)
{
- ImapResponse rc = ir_check_crlf(h, sio_getc(h->sio));
+ ImapResponse rc;
+
+ if (h->state != IMHS_SELECTED) {
+ /* does not make sense in any other state, see RFC 3501, Sect. 6.4. and 7.4.1. */
+ g_info("received EXPUNGE response in bad state %d", h->state);
+ return IMR_PROTOCOL;
+ }
+
+ if (seqno > h->exists) {
+ g_info("received EXPUNGE %u response with only %u messages", seqno, h->exists);
+ return IMR_PROTOCOL;
+ }
+
+ rc = ir_check_crlf(h, sio_getc(h->sio));
g_signal_emit(h, imap_mbox_handle_signals[EXPUNGE_NOTIFY],
0, seqno);
+ /* Current code guarantees that h->flag_cache->len == h->exists, so
+ * the above guard means that it is safe to remove seqno - 1 from
+ * h->flag_cache; we will assert that, to catch any changes in the
+ * future: */
+ g_assert(h->flag_cache->len == h->exists);
g_array_remove_index(h->flag_cache, seqno-1);
+
+ /* Similarly, current code guarantees that h->msg_cache is allocated
+ * at least h->exists elements, so it is safe to dereference
+ * h->msg_cache[seqno - 1]; however, it is a plain C array, so we
+ * cannot check that here, just keep our fingers crossed: */
if(h->msg_cache[seqno-1] != NULL)
imap_message_free(h->msg_cache[seqno-1]);
while(seqno<h->exists) {
@@ -3981,6 +4022,12 @@ ir_fetch_seq(ImapMboxHandle *h, unsigned seqno)
int c = 0;
ImapResponse rc;
+ if (h->state != IMHS_SELECTED) {
+ /* bad state for a response to FETCH, see RFC 3501, Sect. 6.4. and 7.4.2. */
+ g_info("received FETCH response in bad state %d", h->state);
+ return IMR_PROTOCOL;
+ }
+
if(seqno<1 || seqno > h->exists) return IMR_PROTOCOL;
if(sio_getc(h->sio) != '(') return IMR_PROTOCOL;
do {
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]