Package: cyrus-imapd
Version: 3.6.1-2
Severity: important
Tags: patch
User: ubuntu-de...@lists.ubuntu.com
Usertags: origin-ubuntu lunar ubuntu-patch
Forwarded: https://github.com/cyrusimap/cyrus-imapd/pull/4433

Dear maintainers,

In Ubuntu, we discovered by way of a build-time test suite failure that
cyrus-imapd makes some calls to snprintf() with incorrect arguments that
lead to incorrect runtime behavior against glibc 2.37.

Specifically, an indirect caller passes INT_MAX as the size of a buffer of
unknown length, causing glibc's snprintf() implementation to wig out and not
copy the final byte of the source string.

This is not a serious bug for Debian right now because Debian is currently
shipping glibc 2.36, but will eventually become RC.

Thanks for considering,
-- 
Steve Langasek                   Give me a lever long enough and a Free OS
Debian Developer                   to set it on, and I can move the world.
Ubuntu Developer                                   https://www.debian.org/
slanga...@ubuntu.com                                     vor...@debian.org
diff -Nru cyrus-imapd-3.6.1/debian/patches/no-INT_MAX-snprintf.patch 
cyrus-imapd-3.6.1/debian/patches/no-INT_MAX-snprintf.patch
--- cyrus-imapd-3.6.1/debian/patches/no-INT_MAX-snprintf.patch  1969-12-31 
16:00:00.000000000 -0800
+++ cyrus-imapd-3.6.1/debian/patches/no-INT_MAX-snprintf.patch  2023-03-15 
08:11:32.000000000 -0700
@@ -0,0 +1,46 @@
+Description: don't let callers pass INT_MAX as a length to snprintf()
+ There's a lot of code history here that leads to callers passing 
+ buffers two levels down through the code to snprintf() without knowing 
+ their size and therefore passing (approximately) INT_MAX as a length 
+ argument to snprintf().
+ .
+ This is clearly incorrect but has worked up until now since the data 
+ written falls well within the actual size of the buffer so there are no 
+ buffer overflows.  But changes in how boundary checking is implemented 
+ in glibc 2.37 mean this is now failing on (at least) armhf.
+ .
+ Instead of telling snprintf() we have unlimited space in which to write our
+ two characters, set an upper bound so snprintf() knows we need NO MORE THAN
+ two bytes.
+ .
+ This is a workaround for the incorrect length argument still being passed in
+ from up the stack.  A correct solution would fix the API to not let callers
+ pass buffers of undeclared length down into string manipulation functions
+ and use the overflow protection features of the language instead of relying
+ on out-of-band promises in code comments that buffers are "big enough".
+Bug-Ubuntu: https://bugs.launchpad.net/bugs/2011326
+Author: Steve Langasek <steve.langa...@ubuntu.com>
+Last-Update: 2023-03-15
+Forwarded: https://github.com/cyrusimap/cyrus-imapd/pull/4433
+
+Index: cyrus-imapd-3.6.1/lib/times.c
+===================================================================
+--- cyrus-imapd-3.6.1.orig/lib/times.c
++++ cyrus-imapd-3.6.1/lib/times.c
+@@ -45,6 +45,7 @@
+ #include <stdio.h>
+ #include <string.h>
+ #include <strings.h>
++#include <sys/param.h>
+ 
+ #include "assert.h"
+ #include "times.h"
+@@ -562,7 +563,7 @@
+ 
+         /* UTC can be written "Z" or "+00:00" */
+         if (gmtoff == 0)
+-            rlen += snprintf(buf+rlen, len-rlen, "Z");
++            rlen += snprintf(buf+rlen, MIN(len-rlen, 2), "Z");
+         else
+             rlen += snprintf(buf+rlen, len-rlen, "%c%.2lu:%.2lu",
+                              gmtnegative ? '-' : '+', gmtoff/60, gmtoff%60);
diff -Nru cyrus-imapd-3.6.1/debian/patches/series 
cyrus-imapd-3.6.1/debian/patches/series
--- cyrus-imapd-3.6.1/debian/patches/series     2023-02-12 18:50:58.000000000 
-0800
+++ cyrus-imapd-3.6.1/debian/patches/series     2023-03-15 07:49:36.000000000 
-0700
@@ -8,3 +8,4 @@
 0018-increase-test-timeout.patch
 #0019-propagate-XXFLAGS.patch
 0020_fix-cyr_cd-shebang.patch
+no-INT_MAX-snprintf.patch

Reply via email to