View Issue Details

Jump to NotesJump to History
IDProjectCategoryView StatusDate SubmittedLast Update
0005734SOGoSOPEpublic2023-04-10 15:412023-04-17 21:31
Reporterjam Assigned Tosebastien  
PrioritynormalSeveritycrashReproducibilityalways
Status resolvedResolutionfixed 
PlatformServer [Linux]OSGentoo LinuxOS Version17.1/hardened
Product Version5.8.2 
Fixed in Version5.8.3 
Summary0005734: Buffer Overflow in WOHttpTransaction
Description

After updating my system and SOPE and SOGo to 5.8.2, there's a buffer overflow thrown with each HTTP request.

The following is logged (with debug enabled):
Apr 10 16:44:11 sogod [25728]: <0x0x55a43e9bf1b0[WOHttpAdaptor]> notified the watchdog that we are ready
Apr 10 16:44:16 sogod [25728]: |SOGo| starting method 'GET' on uri '/SOGo/'
Apr 10 16:44:16 sogod [25728]: <0x0x55a43e70c9e0[SOGoCache]> Cache cleanup interval set every 300.000000 seconds
Apr 10 16:44:16 sogod [25728]: <0x0x55a43e70c9e0[SOGoCache]> Using host(s) 'localhost' as server(s)
2023-04-10 16:44:16.402 sogod[25728:25728] <MySQL4Channel[0x0x55a43e988980] connection=0x0x55a43e6f6cf0> SQL: BEGIN;
2023-04-10 16:44:16.402 sogod[25728:25728] <MySQL4Channel[0x0x55a43e988980] connection=0x0x55a43e6f6cf0> query has no results.
2023-04-10 16:44:16.402 sogod[25728:25728] <MySQL4Channel[0x0x55a43e988980] connection=0x0x55a43e6f6cf0> SQL: SELECT t1.c_creationdate, t1.c_id, t1.c_lastseen, t1.c_value FROM sogo_sessions_folder t1 WHERE t1.c_id='G0PcoWmXgZfgTqWY';
2023-04-10 16:44:16.402 sogod[25728:25728] <MySQL4Channel[0x0x55a43e988980] connection=0x0x55a43e6f6cf0> query has results, entering fetch-mode.
2023-04-10 16:44:16.402 sogod[25728:25728] <MySQL4Channel[0x0x55a43e988980] connection=0x0x55a43e6f6cf0> SQL: ROLLBACK;
2023-04-10 16:44:16.402 sogod[25728:25728] <MySQL4Channel[0x0x55a43e988980] connection=0x0x55a43e6f6cf0> query has no results.
2023-04-10 16:44:16.403 sogod[25728:25728] MySQL4 connection established 0x0x55a43e9e4740
2023-04-10 16:44:16.403 sogod[25728:25728] ---------- -[MySQL4Channel openChannel]: <MySQL4Channel[0x0x55a43ea48770] connection=0x0x55a43e9e4740> opens channel count[1]
2023-04-10 16:44:16.403 sogod[25728:25728] MySQL4 channel 0x0x55a43ea48770 opened (connection=0x0x55a43e9e4740,sogo)
2023-04-10 16:44:16.403 sogod[25728:25728] <MySQL4Channel[0x0x55a43ea48770] connection=0x0x55a43e9e4740> SQL: BEGIN;
2023-04-10 16:44:16.403 sogod[25728:25728] <MySQL4Channel[0x0x55a43ea48770] connection=0x0x55a43e9e4740> query has no results.
2023-04-10 16:44:16.403 sogod[25728:25728] <MySQL4Channel[0x0x55a43ea48770] connection=0x0x55a43e9e4740> SQL: SELECT t1.c_creationdate, t1.c_id, t1.c_lastseen, t1.c_value FROM sogo_sessions_folder t1 WHERE t1.c_id='G0PcoWmXgZfgTqWY';
2023-04-10 16:44:16.403 sogod[25728:25728] <MySQL4Channel[0x0x55a43ea48770] connection=0x0x55a43e9e4740> query has results, entering fetch-mode.
2023-04-10 16:44:16.403 sogod[25728:25728] <MySQL4Channel[0x0x55a43ea48770] connection=0x0x55a43e9e4740> SQL: ROLLBACK;
2023-04-10 16:44:16.403 sogod[25728:25728] <MySQL4Channel[0x0x55a43ea48770] connection=0x0x55a43e9e4740> query has no results.
2023-04-10 16:44:16.403 sogod[25728:25728] <MySQL4Channel[0x0x55a43e988980] connection=0x0x55a43e6f6cf0> SQL: BEGIN;
2023-04-10 16:44:16.403 sogod[25728:25728] <MySQL4Channel[0x0x55a43e988980] connection=0x0x55a43e6f6cf0> query has no results.
2023-04-10 16:44:16.403 sogod[25728:25728] <MySQL4Channel[0x0x55a43e988980] connection=0x0x55a43e6f6cf0> SQL: UPDATE sogo_sessions_folder SET c_lastseen = 1681137856, c_creationdate = 1676146453, c_value = '<REDACTED>', c_id = 'G0PcoWmXgZfgTqWY' WHERE c_id='G0PcoWmXgZfgTqWY';
2023-04-10 16:44:16.403 sogod[25728:25728] <MySQL4Channel[0x0x55a43e988980] connection=0x0x55a43e6f6cf0> query has no results.
2023-04-10 16:44:16.403 sogod[25728:25728] <MySQL4Channel[0x0x55a43e988980] connection=0x0x55a43e6f6cf0> SQL: COMMIT;
2023-04-10 16:44:16.404 sogod[25728:25728] <MySQL4Channel[0x0x55a43e988980] connection=0x0x55a43e6f6cf0> query has no results.
Apr 10 16:44:16 sogod [25728]: [WARN] <0x0x7fba66c916a0[WOxElemBuilder]> could not locate builders: WOxExtElemBuilder,WOxExtElemBuilder
Apr 10 16:44:16 sogod [25728]: |SOGo| constructed root-url: /SOGo/
Apr 10 16:44:16 sogod [25728]: |SOGo| setting root-url in context: /SOGo/
Apr 10 16:44:16 sogod [25728]: |SOGo| ROOT baseURL(no container, name=(null)):
own: /SOGo/
Apr 10 16:44:16 sogod [25728]: |SOGo| request took 0.010906 seconds to execute
buffer overflow detected : terminated
Apr 10 16:44:16 sogod [25472]: <0x0x55a43e996ce0[WOWatchDogChild]> child 25728 exited
Apr 10 16:44:16 sogod [25472]: <0x0x55a43e996ce0[WOWatchDogChild]> (terminated due to signal 6)
Apr 10 16:44:16 sogod [25472]: <0x0x55a43e996ce0[WOWatchDogChild]> avoiding to respawn child before 2023-04-10 16:44:16 +0200
Apr 10 16:44:17 sogod [25472]: <0x0x55a43e8cefd0[WOWatchDog]> child spawned with pid 25729

The stack trace from the debugger looks as follows:
Program received signal SIGABRT, Aborted.
0x00007fba660acd0c in ?? () from /lib64/libc.so.6
(gdb) bt
#0 0x00007fba660acd0c in ?? () from /lib64/libc.so.6
0000001 0x00007fba6605ad96 in raise () from /lib64/libc.so.6
0000002 0x00007fba660447fc in abort () from /lib64/libc.so.6
0000003 0x00007fba660a0666 in ?? () from /lib64/libc.so.6
0000004 0x00007fba6613e0d2 in fortify_fail () from /lib64/libc.so.6
0000005 0x00007fba6613c9b6 in __chk_fail () from /lib64/libc.so.6
0000006 0x00007fba6613c5d5 in snprintf_chk () from /lib64/libc.so.6
0000007 0x00007fba66ba530b in snprintf (fmt=0x7fba66bed7ba " %i %s\r\n", n=1024, __s=0x7ffc44b4a2b8 "")
at /usr/include/bits/stdio2.h:54
0000008 -[WOHttpTransaction deliverResponse:toRequest:onStream:] (self=0x55a43ea08830, _cmd=<optimized out>,
_response=0x55a43e8fcdc0, _request=0x55a43e7575f0, _out=0x55a43e855200) at WOHttpTransaction.m:752
0000009 0x00007fba66ba2da5 in -[WOHttpTransaction _sendResponse] (self=0x55a43ea08830, _cmd=<optimized out>)
at WOHttpTransaction.m:431
0000010 0x00007fba66ba38ab in -[WOHttpTransaction _run] (self=0x55a43ea08830, _cmd=<optimized out>)
at WOHttpTransaction.m:584
0000011 0x00007fba66ba4c5a in -[WOHttpTransaction run] (self=0x55a43ea08830, _cmd=<optimized out>)
at WOHttpTransaction.m:619
0000012 0x00007fba66b9feb2 in -[WOHttpAdaptor runConnection:] (self=0x55a43eb15fe0, _cmd=<optimized out>,
_socket=<optimized out>) at WOHttpAdaptor.m:373
0000013 0x00007fba66ba0e8c in -[WOHttpAdaptor _handleAcceptedConnection:] (self=self@entry=0x55a43eb15fe0,
_cmd=_cmd@entry=0x7fba66cc9d20 <_OBJC_SELECTOR_TABLE+1344>, _connection=<optimized out>) at WOHttpAdaptor.m:407
0000014 0x00007fba66ba11c0 in -[WOHttpAdaptor _handleConnection:] (self=0x55a43eb15fe0, _cmd=<optimized out>,
connection=0x55a43e8fcfb0) at WOHttpAdaptor.m:466
0000015 0x00007fba66ba0222 in -[WOHttpAdaptor acceptControlMessage:] (self=0x55a43eb15fe0, _cmd=<optimized out>,
aNotification=<optimized out>) at WOHttpAdaptor.m:505
0000016 0x00007fba664bac24 in ?? () from /usr/lib64/libgnustep-base.so.1.27
0000017 0x00007fba665d197e in ?? () from /usr/lib64/libgnustep-base.so.1.27
0000018 0x00007fba66506e02 in ?? () from /usr/lib64/libgnustep-base.so.1.27
0000019 0x00007fba66506a20 in ?? () from /usr/lib64/libgnustep-base.so.1.27
0000020 0x00007fba66b17376 in -[WOCoreApplication run] (self=0x55a43ead1710, _cmd=<optimized out>)
at WOCoreApplication.m:584
0000021 0x000055a43d907ee4 in -[SOGo run] (self=0x55a43ead1710, _cmd=<optimized out>) at SOGo.m:337
0000022 0x00007fba66b55bbb in -[WOWatchDog _spawnChild:] (self=0x55a43e8cefd0, _cmd=<optimized out>, child=0x55a43e996ce0)
at WOWatchDogApplicationMain.m:600
0000023 0x00007fba66b54bad in -[WOWatchDog _ensureChildren] (self=0x55a43e8cefd0, _cmd=<optimized out>)
at WOWatchDogApplicationMain.m:690
0000024 0x00007fba66b56635 in -[WOWatchDog run:argc:argv:] (self=0x55a43e8cefd0, _cmd=<optimized out>,
newAppName=<optimized out>, newArgC=<optimized out>, newArgV=<optimized out>) at WOWatchDogApplicationMain.m:942
0000025 0x00007fba66b56b82 in WOWatchDogApplicationMain (appName=appName@entry=0x55a43d90f200 <_OBJC_INSTANCE_3.1>,
argc=argc@entry=8, argv=argv@entry=0x7ffc44b4f5e8) at WOWatchDogApplicationMain.m:1051
0000026 0x000055a43d9062f7 in main (argc=8, argv=0x7ffc44b4f5e8, env=<optimized out>) at sogod.m:51

The source code in WOHttpTransaction.m:752 shows a buggy call to snprintf:
snprintf((char *)&(buf[slen]), sizeof(buf), " %i %s\r\n", s, r);

The size of "sizeof(buf)" is the full buffer size, but the address given to snprintf isn't the start of the buffer.
As a consequence, that call might actually write behind the end of the buffer.

(gdb) f 8
0000008 -[WOHttpTransaction deliverResponse:toRequest:onStream:] (self=0x55a43ea08830, _cmd=<optimized out>,
_response=0x55a43e8fcdc0, _request=0x55a43e7575f0, _out=0x55a43e855200) at WOHttpTransaction.m:752
752 in WOHttpTransaction.m
(gdb) p slen
$1 = 8

Steps To Reproduce

Navigate your browser to the SOGo login page. Described error is thrown.

Additional Information

System uname: Linux-6.1.19-gentoo-x86_64-x86_64-AMD_Ryzen_5_5600G_with_Radeon_Graphics-with-glibc2.36
sh bash 5.1_p16-r2
ld GNU ld (Gentoo 2.39 p5) 2.39.0
app-misc/pax-utils: 1.3.5::gentoo
app-shells/bash: 5.1_p16-r2::gentoo
dev-lang/perl: 5.36.0-r2::gentoo
dev-lang/python: 3.10.10_p3::gentoo, 3.11.2_p2::gentoo
dev-lang/rust: 1.66.1::gentoo
dev-util/cmake: 3.25.3::gentoo
dev-util/meson: 1.0.1::gentoo
sys-apps/baselayout: 2.13-r1::gentoo
sys-apps/openrc: 0.46::gentoo
sys-apps/sandbox: 2.29::gentoo
sys-devel/autoconf: 2.71-r5::gentoo
sys-devel/automake: 1.16.5::gentoo
sys-devel/binutils: 2.39-r4::gentoo
sys-devel/binutils-config: 5.5::gentoo
sys-devel/gcc: 12.2.1_p20230121-r1::gentoo
sys-devel/gcc-config: 2.10::gentoo
sys-devel/libtool: 2.4.7-r1::gentoo
sys-devel/make: 4.3::gentoo
sys-kernel/linux-headers: 6.1::gentoo (virtual/os-headers)
sys-libs/glibc: 2.36-r7::gentoo

Reverting to SOGo 5.7.1 (the previously installed version) didn't help. I'm not totally sure as of now why the system is sending SIGABRT just now.
Problem is, that there were quite some updates and changes to the system since the installation of SOGo 5.7.1 in 09/2022.

TagsNo tags attached.

Activities

jam

jam

2023-04-10 16:09

reporter   ~0016818

The following patch fixed it for me:

--- a/sope-appserver/NGObjWeb/WOHttpAdaptor/WOHttpTransaction.m 2023-04-10 17:56:48.098843793 +0200
+++ b/sope-appserver/NGObjWeb/WOHttpAdaptor/WOHttpTransaction.m 2023-02-27 22:01:49.000000000 +0100
@@ -749,7 +749,7 @@
       rlen = strlen((const char *)r);
       if ((slen + rlen + 8) &lt; 1000) {
         [t1 getCString:(char *)buf]; // HTTP status
-        snprintf((char *)&(buf[slen]), sizeof(buf), &quot; %i %s\r\n&quot;, s, r);
+        snprintf((char *)&(buf[slen]), sizeof(buf)-slen, &quot; %i %s\r\n&quot;, s, r);
         isok = [_out safeWriteBytes:buf count:strlen((char *)buf)];
       }
       else

... it might probably be better to add some checks here. this patch is just meant to get my install working again asap.
sebastien

sebastien

2023-04-17 13:36

administrator   ~0016833

That's strange, however you're right. I would change this :

--- i/sope-appserver/NGObjWeb/WOHttpAdaptor/WOHttpTransaction.m
+++ w/sope-appserver/NGObjWeb/WOHttpAdaptor/WOHttpTransaction.m
@@ -738,18 +738,20 @@ - (void)deliverResponse:(WOResponse *)_response
     if (isok) {
       unsigned int slen, rlen;
       const unsigned char *r;
-      int s;
+      int s, size;

       s  = [_response status];
       t1 = [_response httpVersion];
       r  = [self _reasonForStatus:s];
+      size = 7 + strlen(r); // Size of status (e.g. : 200 OK \r\n) - 1 space, 3 digits for status code, 1 space, X for status, 2 end of line

       // TBD: replace -cStringLength/-getCString:
       slen = [t1 cStringLength];
       rlen = strlen((const char *)r);
-      if ((slen + rlen + 8) &lt; 1000) {
+      if (((slen + rlen + 8) &lt; 1000) 
+        && ((slen + size) &lt; sizeof(buf))) {
         [t1 getCString:(char *)buf]; // HTTP status
-        snprintf((char *)&(buf[slen]), sizeof(buf), &quot; %i %s\r\n&quot;, s, r);
+        snprintf((char *)&(buf[slen]), size, &quot; %i %s\r\n&quot;, s, r);
         isok = [_out safeWriteBytes:buf count:strlen((char *)buf)];
       }
       else

Can you confirm this is ok for you and works ?

Sebastien
jam

jam

2023-04-17 17:53

reporter   ~0016839

Yes, I can confirmed that your patch works. sogod is not getting shot by SIGABRT.
But: Reviewing this patch I'm not sure about the trailing null byte.
That code is passing exactly the string size to snprintf, not leaving space for the trailing null byte.

From the snprintf man page:

The functions snprintf() and vsnprintf() do not write more than size bytes  (including  the  terminating  null
byte  ('\0')). If the output was truncated due to this limit, then the return value is the number of characters
(excluding the terminating null byte) which would have been written to the final string if  enough  space
had  been available.

So depending on the implementation of snprintf, the output is either truncated by a byte and the null byte is written, or the null byte will be omitted.
The count of 8 bytes in the existing if-statement suggests that somebody originally thought about that.
My machine is opting for the first, safer option and "eats up" the \n:

(gdb) p buf
$5 = &quot;HTTP/1.1 200 OK\r\000%H:%M\&quot;,\&quot;SOGoRem&quot;, '\000' &lt;repeats 12 times>, &lt;REDACTED>

I'd like to propose a patch that simplifies the code passage a little and accounts for the trailing null byte.

--- a/sope-appserver/NGObjWeb/WOHttpAdaptor/WOHttpTransaction.m 2023-02-27 22:01:49.000000000 +0100
+++ b/sope-appserver/NGObjWeb/WOHttpAdaptor/WOHttpTransaction.m 2023-04-17 19:12:50.576522017 +0200
@@ -738,7 +738,7 @@
     if (isok) {
       unsigned int slen, rlen;
       const unsigned char *r;
-      int s;
+      int s, size;

       s  = [_response status];
       t1 = [_response httpVersion];
@@ -747,9 +747,10 @@
       // TBD: replace -cStringLength/-getCString:
       slen = [t1 cStringLength];
       rlen = strlen((const char *)r);
-      if ((slen + rlen + 8) &lt; 1000) {
+      size = 8 + rlen; // Size of status (e.g. : 200 OK \r\n) - 1 space, 3 digits for status code, 1 space, X for status, 2 end of line, 1 for zero-byte
+      if ((slen + size) &lt; sizeof(buf)) {
         [t1 getCString:(char *)buf]; // HTTP status
-        snprintf((char *)&(buf[slen]), sizeof(buf), &quot; %i %s\r\n&quot;, s, r);
+        snprintf((char *)&(buf[slen]), size, &quot; %i %s\r\n&quot;, s, r);
         isok = [_out safeWriteBytes:buf count:strlen((char *)buf)];
       }
       else

Changes are:

  • size is computed like in your patch, but using the determined rlen AND ending null byte
  • the if statement only checks if there is enough space in buf for "HTTP/1.1 200 OK \r\n\0"
  • snprintf takes size instead of sizeof(buf)

With this patch the results in buf are correct (in a sense that all bytes are being written to it):

(gdb) p buf
$3 = &quot;HTTP/1.1 200 OK\r\n\000aWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=Gy5BaoH1NTIeXmFPa2tf4sq8S3V86g%2FXTbiUb9Yj81w%3D&reserved=0\\\&quot;>https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugs.s&quot;...
sebastien

sebastien

2023-04-17 21:31

administrator   ~0016841

I'm ok, you're right the trailing null byte. For the rest of the code it is likely the same and simplify the condition.

Commit : https://github.com/Alinto/sope/commit/879cfd14fbd912a6096fda68874e9a93611297c8

Sebastien

Issue History

Date Modified Username Field Change
2023-04-10 15:41 jam New Issue
2023-04-10 16:09 jam Note Added: 0016818
2023-04-17 13:36 sebastien Note Added: 0016833
2023-04-17 13:36 sebastien Assigned To => sebastien
2023-04-17 13:36 sebastien Status new => feedback
2023-04-17 17:53 jam Note Added: 0016839
2023-04-17 17:53 jam Status feedback => assigned
2023-04-17 21:31 sebastien Note Added: 0016841
2023-04-17 21:31 sebastien Status assigned => resolved
2023-04-17 21:31 sebastien Resolution open => fixed
2023-04-17 21:31 sebastien Fixed in Version => 5.8.3