View Issue Details

Jump to NotesJump to History
IDProjectCategoryView StatusDate SubmittedLast Update
0005631SOGowith SOGopublic2022-10-23 00:492022-10-23 10:23
Reportermtbro Assigned To 
PrioritynormalSeveritycrashReproducibilityalways
Status newResolutionopen 
Product Version5.7.0 
Summary0005631: FreeBSD; sogod crashing ; stack smashing detected; off by one
Description

setup: FreeBSD 13.1, sogo 5.7.2, pgsql 13.8.
issue: Users are unable to login, sogod reporting:

2022-10-23 02:51:55.023 sogod[55773:100929] PG0x0x804872368 SQL: SELECT c_password FROM sogo_view WHERE c_uid = 'sogo'
Oct 23 02:51:55 sogod [55764]: [ERROR] <0x0x803e89818[WOWatchDog]> No child available to handle incoming request!
Oct 23 02:51:55 sogod [55764]: <0x0x8048c80a8[WOWatchDogChild]> child 55773 exited
Oct 23 02:51:55 sogod [55764]: <0x0x8048c80a8[WOWatchDogChild]> (terminated due to signal 6)

Live debugging (gdb attached to sogod) showed crash at decodeDataFromHexString(). Canary on stack was smashed exactly by one byte with NULL byte. Further checks showed that issue is here:

https://github.com/Alinto/sogo/blob/master/SoObjects/SOGo/NSData%2BCrypto.m#L123

I don't have much experience with objective-c, quick google search on getCString() got me here: https://developer.apple.com/documentation/foundation/nsstring/1415702-getcstring
It seems the older versions required for buffer to have NULL byte in. What is older version and how do I know it's that, I don't know.

Anyway FreeBSD doesn't have NULL byte in its canary as Linux does. This issue could have been undetected on Linux boxes as byte 0x0 is rewritten to 0x0 byte (at least on little endian systems) there.

Fix that I've tested and works was just to make space for that extra byte:

[theString getCString:(char *)srcBuffer+1];

Steps To Reproduce

Install and configure sogo on FreeBSD, try to login.

Additional Information

diff --git a/SoObjects/SOGo/NSData+Crypto.m b/SoObjects/SOGo/NSData+Crypto.m
index 6a2f241d1..9d3f0e08d 100644
--- a/SoObjects/SOGo/NSData+Crypto.m
+++ b/SoObjects/SOGo/NSData+Crypto.m
@@ -119,7 +119,7 @@ static const char salt_chars[] =
unsigned int stringLength = [theString length];
unsigned int byteLength = stringLength/2;
unsigned int byteCounter = 0;

  • unsigned char srcBuffer[stringLength];
  • unsigned char srcBuffer[stringLength+1];
    [theString getCString:(char )srcBuffer];
    unsigned char     srcPtr = srcBuffer;
    unsigned char dstBuffer[byteLength];
TagsPatch

Activities

mtbro

mtbro

2022-10-23 10:23

reporter   ~0016307

I'm attaching the patch as in desc patch got modified.
In the description I made a mistake, the line that I've changed is

unsigned char srcBuffer[stringLength+1];

sogopatch.patch (576 bytes)    
diff --git a/SoObjects/SOGo/NSData+Crypto.m b/SoObjects/SOGo/NSData+Crypto.m
index 6a2f241d1..9d3f0e08d 100644
--- a/SoObjects/SOGo/NSData+Crypto.m
+++ b/SoObjects/SOGo/NSData+Crypto.m
@@ -119,7 +119,7 @@ static const char salt_chars[] =
   unsigned int stringLength = [theString length];
   unsigned int byteLength = stringLength/2;
   unsigned int byteCounter = 0;
-  unsigned char srcBuffer[stringLength];
+  unsigned char srcBuffer[stringLength+1];
   [theString getCString:(char *)srcBuffer];
   unsigned char *srcPtr = srcBuffer;
   unsigned char dstBuffer[byteLength];
sogopatch.patch (576 bytes)   

Issue History

Date Modified Username Field Change
2022-10-23 00:49 mtbro New Issue
2022-10-23 00:49 mtbro Tag Attached: Patch
2022-10-23 10:23 mtbro Note Added: 0016307
2022-10-23 10:23 mtbro File Added: sogopatch.patch