Hello,
I found a DoS bug in SOPE, the set of Objective-C frameworks powering
SOGo [1].
It is way too easy to trigger with curl:
curl -d 'x=' 'https://.../SOGo/?x='
As far as I can tell all versions since SOGo-2.0.2 (2012-10-29) are
affected.
# Details
I found it inspecting multiple SOGo crashes triggered by someone
hitting URLs like `/OA_HTML/BneViewerXMLService?bne:uueupload=TRUE`
with `POST` that my search engine associates with "Oracle E-Business
Suite (EBS)".
The crash happens in NGHashMap.m line 790 [2], as root->last is NULL:
root->last->next = element;
NGHashMap maps a key to a single-linked list of values.
It turns out `-[NGMutableHashMap addObjects:count:forKey:]` is the
only function maintaining the `root->last` pointer; everything else in
this file doesn't touch it.
The SOPE request handling first parses the POST body for
"formParameters", then clones that NGHashMap (the copied nodes have
`last == NULL`), and then merges the query string parameters into it;
if there is a duplicate key the bug triggers.
I'd like to point out that the linked-list implementation is quite
bad. It wastes memory by using the same struct for the root node
(with count and last metadata) and the member nodes, and seems to
throw exceptions (e.g. when values are nil) but isn't exception
safe (metadata isn't updated consistently).
See attached patch for an attempt to maintain the `last` pointer
properly across all methods; it seems to work for me (on top of
5.8.0-1 in debian/bookworm); submitted to upstream in [4].
In the long run at least the linked-list implementation should
probably be replaced by using some properly tested library.
The `last` handling was introduced in dfceefc 2012-10-15 [3], and I
think it has been broken since then.
(I haven't actually tried to reproduce it with latest upstream, but I
don't think any of the code has been touched in relevant ways.)
cheers,
Stefan
[1] https://www.sogo.nu/
[2]
https://github.com/Alinto/sope/blob/3146fbdb6ff3314e37e5c3682deeeef7d0f32064/sope-core/NGExtensions/NGHashMap.m#L790
[3]
https://github.com/Alinto/sope/commit/dfceefcb141c1b31b26eea19ca07d3916d663315
[4] https://github.com/Alinto/sope/pull/69From 280104e45c20519ac4849ebf8bca114d91383543 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Stefan=20B=C3=BChler?= <sou...@stbuehler.de>
Date: Sun, 29 Jun 2025 10:21:32 +0200
Subject: [PATCH] NGHashMap: keep root->last consistent to fix segfault adding
duplicate key after copy
segfault because root->last is NULL:
#0 -[NGMutableHashMap addObjects:count:forKey:]
#1 -[NGMutableHashMap addObject:forKey:]
#2 -[NGHttpRequest(WOSupport) _decodeFormContentURLParameters:]
#3 -[NGHttpRequest(WOSupport) formParameters]
when POST and GET set the same parameter; trigger like this:
curl -d 'x=' 'https://.../SOGo/?x='
---
sope-core/NGExtensions/NGHashMap.m | 33 ++++++++++++++++++++++--------
1 file changed, 25 insertions(+), 8 deletions(-)
diff --git a/sope-core/NGExtensions/NGHashMap.m b/sope-core/NGExtensions/NGHashMap.m
index 8b05ebb..f8df722 100644
--- a/sope-core/NGExtensions/NGHashMap.m
+++ b/sope-core/NGExtensions/NGHashMap.m
@@ -216,6 +216,7 @@ static inline unsigned __countObjectsForKey(NGHashMap *self, id _key) {
NSEnumerator *keys = nil;
id key = nil;
LList *list = NULL;
+ LList *root = NULL;
LList *newList = NULL;
LList *oldList = NULL;
@@ -223,7 +224,7 @@ static inline unsigned __countObjectsForKey(NGHashMap *self, id _key) {
keys = [_hashMap keyEnumerator];
while ((key = [keys nextObject])) {
list = [_hashMap __structForKey:key];
- newList = initLListElement(list->object,NULL);
+ root = newList = initLListElement(list->object,NULL);
newList->count = list->count;
NSMapInsert(self->table,key,newList);
while (list->next) {
@@ -232,6 +233,7 @@ static inline unsigned __countObjectsForKey(NGHashMap *self, id _key) {
newList = initLListElement(list->object,NULL);
oldList->next = newList;
}
+ root->last = newList;
}
}
return self;
@@ -257,6 +259,7 @@ static inline unsigned __countObjectsForKey(NGHashMap *self, id _key) {
pred = element;
}
root->count = i;
+ root->last = pred;
NSMapInsert(self->table,_key, root);
}
NSAssert(self->table, @"missing table for hashmap ..");
@@ -712,6 +715,7 @@ static inline unsigned __countObjectsForKey(NGHashMap *self, id _key) {
{
id object = nil;
LList *root = NULL;
+ LList *insert = NULL;
LList *element = NULL;
unsigned i = 0;
@@ -728,10 +732,13 @@ static inline unsigned __countObjectsForKey(NGHashMap *self, id _key) {
}
root = initLListElement(_objects[0], NULL);
+ insert = root;
root->count = _count;
+ // set root->last to last inserted element later
NSMapInsert(self->table, _key, root);
}
else {
+ insert = root;
if (!(_index < root->count)) {
[NSException raise:NSRangeException
format:@"index %"PRIuPTR" out of range in map 0x%p length %d",
@@ -741,30 +748,38 @@ static inline unsigned __countObjectsForKey(NGHashMap *self, id _key) {
root->count += _count;
if (_index == 0) {
+ // move current data at pos 0 to new element (prepending
+ // a new element would require replacing entry in NSMapTable)
element = initLListElement(_objects[0],NULL);
object = element->object;
element->next = root->next;
element->object = root->object;
root->object = object;
root->next = element;
+ if (root->last == root)
+ root->last = element; // inserted at pos 0 before the only item
}
else {
while (--_index)
- root = root->next;
+ insert = insert->next;
+ if (root->last == insert)
+ root->last = NULL; // set to last inserted element later
element = initLListElement(_objects[0], NULL);
- element->next = root->next;
- root->next = element;
- root = root->next;
+ element->next = insert->next;
+ insert->next = element;
+ insert = insert->next;
}
}
for (i = 1; i < _count; i++) {
checkForAddErrorMessage(self, _objects[i], _key);
element = initLListElement(_objects[i], NULL);
- element->next = root->next;
- root->next = element;
- root = element;
+ element->next = insert->next;
+ insert->next = element;
+ insert = element;
}
+ if (root->last == NULL)
+ root->last = insert;
}
/* adding objects */
@@ -864,6 +879,7 @@ static inline unsigned __countObjectsForKey(NGHashMap *self, id _key) {
root->next = list->next;
root->object = list->object;
root->count--;
+ // cleanup root->last (could be list!) after loop below
if (list) free(list);
list = NULL;
}
@@ -880,6 +896,7 @@ static inline unsigned __countObjectsForKey(NGHashMap *self, id _key) {
list = oldList;
}
}
+ root->last = list; // list->next is NULL, i.e. it is the last
root->count -= cnt;
}
}
--
2.49.0
