Merge pull request #4910 from cyrusimap/message_patch_binary_cte_header

Fix invalid read for BINARY content-transfer-encoding header
cyrusimap · May 16, 2024 · d5d0cc6 · d5d0cc6
2 parents 930fcd9 + a697983
commit d5d0cc6
Show file tree

Hide file tree

Showing 4 changed files with 56 additions and 9 deletions.
diff --git a/cassandane/Cassandane/Cyrus/Simple.pm b/cassandane/Cassandane/Cyrus/Simple.pm
@@ -280,4 +280,26 @@ sub test_toggleable_debug_logging
     }
 }
 
+sub test_append_binary
+{
+    my ($self) = @_;
+    my $imap = $self->{store}->get_client();
+
+    my $mime = <<'EOF' =~ s/\n/\r\n/gr;
+To: to@local
+From: from@local
+Subject: test
+Content-Transfer-Encoding:binary
+
+test
+EOF
+
+    $imap->append("INBOX", { Binary => $mime });
+    $self->assert_str_equals('ok', $imap->get_last_completion_response());
+
+    $imap->select('INBOX');
+    my $res = $imap->fetch('1', '(BINARY[1])');
+    $self->assert_str_equals("test\r\n", $res->{1}{binary});
+}
+
 1;
diff --git a/imap/message.c b/imap/message.c
@@ -955,13 +955,30 @@ static int message_parse_headers(struct msg *msg, struct body *body,
 
                 /* If we're encoding binary, replace "binary"
                    with "base64" in CTE header body */
-                if (msg->encode &&
-                    !strcmpsafe(body->encoding, "BINARY")) {
-                    char *p = (char*)
-                        stristr(msg->base + body->header_offset +
-                                (next - headers.s) + 27,
-                                "binary");
-                    memcpy(p, "base64", 6);
+                if (msg->encode && !strcmpsafe(body->encoding, "BINARY")) {
+                    // Determine the start and end of the CTE header value
+                    const char *hdr_val = msg->base + body->header_offset +
+                                          (next - headers.s) + 26;
+                    const char *hdr_end = hdr_val;
+                    const char *msghdr_end =
+                        msg->base + body->header_offset + body->header_size;
+                    for (; hdr_end < msghdr_end; hdr_end++) {
+                        if (hdr_end[0] == '\r') {
+                            if (hdr_end + 2 < msghdr_end &&
+                                hdr_end[1] == '\n' &&
+                                hdr_end[2] != ' ' && hdr_end[2] != '\t') {
+                                hdr_end += 2;
+                                break;
+                            }
+                        }
+                    }
+                    // Replace header value
+                    char *p =
+                        (char *)strinstr(hdr_val, hdr_end - hdr_val, "binary");
+                    if (p)
+                        memcpy(p, "base64", 6);
+                    else
+                        xsyslog(LOG_ERR, "can not patch BINARY CTE header", NULL);
                 }
                 break;
             case RFC822_CONTENT_TYPE:

diff --git a/lib/stristr.c b/lib/stristr.c
@@ -27,10 +27,10 @@
  extern "C" {
 #endif
 
-EXPORTED char *stristr(const char *String, const char *Pattern)
+EXPORTED char *strinstr(const char *String, size_t StringLen, const char *Pattern)
 {
       char *pptr, *sptr, *start;
-      size_t slen = strlen(String);
+      size_t slen = StringLen;
       size_t plen = strlen(Pattern);
 
       if (!plen) return (char *)String;
@@ -70,6 +70,11 @@ EXPORTED char *stristr(const char *String, const char *Pattern)
       return(NULL);
 }
 
+EXPORTED char *stristr(const char *String, const char *Pattern)
+{
+      return strinstr(String, strlen(String), Pattern);
+}
+
 #if defined(__cplusplus) && __cplusplus
  }
 #endif
diff --git a/lib/stristr.h b/lib/stristr.h
@@ -4,6 +4,9 @@
 #ifndef INCLUDED_STRISTR_H
 #define INCLUDED_STRISTR_H
 
+#include <stddef.h>
+
 extern char *stristr(const char *haystack, const char *needle);
+extern char *strinstr(const char *haystack, size_t haystack_len, const char *needle);
 
 #endif /* INCLUDED_STRISTR_H */