Skip to content

Commit e3a8366

Browse files
committed
emsmdb: fix use-after-free in ~LOGON_ITEM
LOGON_ITEM::root is going away first, and with it, any information on node order. Then, as phash.~unordered_map runs, it processes object_nodes in arbitrary order, freeing a logon_object before a message_object, but note how ~message_object still wants to use the logon object. ``` ==579226==ERROR: AddressSanitizer: heap-use-after-free on address 0x7d3fa4210ad4 at pc 0x7fcfa5b1a037 bp 0x7ffdf9f81600 sp 0x7ffdf9f815f8 READ of size 1 at 0x7d3fa4210ad4 thread T0 f0 0x7fcfa5b1a036 in gromox::exmdb_client_is_local(char const*, long*) lib/exmdb_client.cpp:438 f1 0x7fcfa8cf86ad in exmdb_client_local::unload_instance(char const*, unsigned int) exch/exmdb/rpc.cpp:991 f2 0x7fcfaabd0c57 in message_object::~message_object() exch/emsmdb/message_object.cpp:183 f3 0x7fcfaae2a22a in object_node::clear() exch/emsmdb/rop_processor.cpp:89 f4 0x7fcfaac05e6e in object_node::~object_node() exch/emsmdb/rop_processor.hpp:35 … f20 0x7fcfaaae8b06 in LOGON_ITEM::~LOGON_ITEM() exch/emsmdb/rop_processor.hpp:20 f21 0x7fcfaaae8b54 in std::default_delete<LOGON_ITEM>::operator()(LOGON_ITEM*) const /usr/include/c++/15/bits/unique_ptr.h:93 f22 0x7fcfaaae703d in std::unique_ptr<LOGON_ITEM>::~unique_ptr() /usr/include/c++/15/bits/unique_ptr.h:399 f23 0x7fcfaaae5b78 in LOGMAP::~LOGMAP() exch/emsmdb/rop_processor.hpp:25 f24 0x7fcfaaae6287 in emsmdb_info::~emsmdb_info() exch/emsmdb/emsmdb_interface.hpp:17 f25 0x7fcfaaab19c0 in ~HANDLE_DATA exch/emsmdb/emsmdb_interface.cpp:306 … f32 0x7fcfaaac64af in clear /usr/include/c++/15/bits/unordered_map.h:862 f33 0x7fcfaaab47b7 in emsmdb_interface_stop() exch/emsmdb/emsmdb_interface.cpp:461 0x7d3fa4210ad4 is located 340 bytes inside of 760-byte region [0x7d3fa4210980,0x7d3fa4210c78) freed by thread T0 here: f0 operator delete(void*, unsigned long) (/lib64/libasan.so.8+0x12382b) f1 object_node::clear() exch/emsmdb/rop_processor.cpp:82 f2 object_node::~object_node() exch/emsmdb/rop_processor.hpp:35 … f26 std::unordered_map<unsigned int, std::shared_ptr<object_node>, std::hash<unsigned int>, std::equal_to<unsigned int>, std::allocator<std::pair<unsigned int const, std::shared_ptr<object_node> > > >::~unordered_map() /usr/include/c++/15/bits/unordered_map.h:112 f27 LOGON_ITEM::~LOGON_ITEM() exch/emsmdb/rop_processor.hpp:20 f28 std::default_delete<LOGON_ITEM>::operator()(LOGON_ITEM*) const /usr/include/c++/15/bits/unique_ptr.h:93 f29 std::unique_ptr<LOGON_ITEM, std::default_delete<LOGON_ITEM> >::~unique_ptr() /usr/include/c++/15/bits/unique_ptr.h:399 f30 LOGMAP::~LOGMAP() exch/emsmdb/rop_processor.hpp:25 f31 emsmdb_info::~emsmdb_info() exch/emsmdb/emsmdb_interface.hpp:17 f32 ~HANDLE_DATA exch/emsmdb/emsmdb_interface.cpp:306 ``` We could reorder LOGON_ITEM members, but if `root` is the last holder of shared_ptr references, there may be a deep call chain for cleanup, which is probably why phash was first. Add back an explicit pre-destructor. Fixes: gromox-1.28-104-g2917d2657
1 parent 873e897 commit e3a8366

File tree

4 files changed

+14
-2
lines changed

4 files changed

+14
-2
lines changed

exch/emsmdb/message_object.cpp

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -179,9 +179,11 @@ message_object::~message_object()
179179
{
180180
auto pmessage = this;
181181

182-
if (pmessage->instance_id != 0)
182+
if (pmessage->instance_id != 0) {
183+
fprintf(stderr, "MO %p references logon %p\n", this, plogon);
183184
exmdb_client->unload_instance(pmessage->plogon->get_dir(),
184185
pmessage->instance_id);
186+
}
185187
if (pmessage->precipient_columns != nullptr)
186188
proptag_array_free(pmessage->precipient_columns);
187189
if (pmessage->pchanged_proptags != nullptr)

exch/emsmdb/message_object.hpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,11 @@ struct ics_state;
1414
struct logon_object;
1515
struct stream_object;
1616

17+
/**
18+
* @plogon: the logon ought to be pinned because object_node<message_object>
19+
* has a shared ptr to the parent (which in turn does, ... so
20+
* the root is held)
21+
*/
1722
struct message_object {
1823
protected:
1924
message_object() = default;

exch/emsmdb/rop_processor.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,13 +79,16 @@ void object_node::clear() noexcept
7979
if (ref != g_logon_hash.end() && --ref->second == 0)
8080
g_logon_hash.erase(ref);
8181
}
82+
fprintf(stderr, "~object_node(%p, parent=%p) deleting inner logon %p\n", this, parent.get(), logon);
8283
delete logon;
8384
break;
8485
}
8586
case ems_objtype::folder:
87+
fprintf(stderr, "~object_node(%p, parent=%p) deleting inner folder %p\n", this, parent.get(), pobject);
8688
delete static_cast<folder_object *>(pobject);
8789
break;
8890
case ems_objtype::message:
91+
fprintf(stderr, "~object_node(%p, parent=%p) deleting inner message %p\n", this, parent.get(), pobject);
8992
delete static_cast<message_object *>(pobject);
9093
break;
9194
case ems_objtype::attach:
@@ -239,6 +242,8 @@ int32_t rop_processor_add_object_handle(LOGMAP *plogmap, uint8_t logon_id,
239242
plogitem->root = pobjnode;
240243
else if (g_emsmdb_full_parenting || object_dep(parent->type, pobjnode->type))
241244
pobjnode->parent = std::move(parent);
245+
fprintf(stderr, "AOH(inner=%p objnode=%p parentnode=%p)\n",
246+
pobjnode->pobject, pobjnode.get(), pobjnode->parent.get());
242247
if (pobjnode->type == ems_objtype::icsupctx) {
243248
auto pemsmdb_info = emsmdb_interface_get_emsmdb_info();
244249
pemsmdb_info->upctx_ref ++;

exch/emsmdb/rop_processor.hpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,8 @@ struct object_node {
3838

3939
uint32_t handle = 0;
4040
ems_objtype type = ems_objtype::none;
41-
void *pobject = nullptr;
4241
std::shared_ptr<object_node> parent;
42+
void *pobject = nullptr;
4343
};
4444

4545
extern void rop_processor_init(int scan_interval);

0 commit comments

Comments
 (0)