Skip to content

Commit

Permalink
ruleproc: repair use-after-free, new[]/free mismatch
Browse files Browse the repository at this point in the history
==2421730==ERROR: AddressSanitizer: alloc-dealloc-mismatch (operator new [] vs free) on 0x602000017b50
f0 __interceptor_free ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:52
f1 propval_free(unsigned short, void*) lib/mapi/propval.cpp:401
f2 tpropval_array_free_internal(TPROPVAL_ARRAY*) lib/mapi/tpropval_array.cpp:112
f3 message_content_free_internal(message_content*) lib/mapi/element_data.cpp:258
f4 message_content_free(message_content*) lib/mapi/element_data.cpp:269
f5 operator() lib/ruleproc.cpp:94
f6 ~unique_ptr /usr/include/c++/12/bits/unique_ptr.h:396
f7 ~rxparam lib/ruleproc.cpp:119
f8 exmdb_local_rules_execute lib/ruleproc.cpp:1266
f9 exmdb_local_deliverquota mda/exmdb_local/exmdb_local.cpp:426
f10 exmdb_local_hook mda/exmdb_local/exmdb_local.cpp:105

0x602000017b50 is located 0 bytes inside of 4-byte region [0x602000017b50,0x602000017b54)
allocated by thread T5 here:
f0 operator new[](unsigned long) ../../../../src/libsanitizer/asan/asan_new_delete.cpp:98
f1 std::__detail::_MakeUniq<char []>::__array std::make_unique<char []>(unsigned long) /usr/include/c++/12/bits/unique_ptr.h:1080
f2 alloc_context::alloc(unsigned long) include/gromox/util.hpp:28
f3 exmdb_local_alloc mda/exmdb_local/exmdb_local.cpp:235
f4 unsigned int* EXT_PULL::anew<unsigned int>() include/gromox/ext_buffer.hpp:219
f5 EXT_PULL::g_propval(unsigned short, void**) lib/mapi/ext_buffer.cpp:1030
f6 EXT_PULL::g_tagged_pv(TAGGED_PROPVAL*) lib/mapi/ext_buffer.cpp:1078
f7 EXT_PULL::g_tpropval_a(TPROPVAL_ARRAY*) lib/mapi/ext_buffer.cpp:1181
f8 EXT_PULL::g_msgctnt(message_content*) lib/mapi/ext_buffer.cpp:2116
f9 exmdb_pull lib/exmdb_ext.cpp:3535
f10 exmdb_ext_pull_response lib/exmdb_ext.cpp:3861
f11 gromox::exmdb_client_do_rpc lib/exmdb_client.cpp:542
f12 gromox::exmdb_client_remote::read_message lib/exmdb_rpc.cpp:1753
f13 run lib/ruleproc.cpp:1222
f14 exmdb_local_rules_execute lib/ruleproc.cpp:1265
f15 exmdb_local_deliverquota mda/exmdb_local/exmdb_local.cpp:426
f16 exmdb_local_hook mda/exmdb_local/exmdb_local.cpp:105

Fixes: gromox-2.41-54-g7ace89b32
References: GXL-581, GXF-2013
  • Loading branch information
jengelh committed Feb 25, 2025
1 parent 1908937 commit ec8e477
Showing 1 changed file with 7 additions and 9 deletions.
16 changes: 7 additions & 9 deletions lib/ruleproc.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -119,20 +119,18 @@ using message_content_ptr = std::unique_ptr<MESSAGE_CONTENT, rx_delete>;
struct rxparam {
/**
* @store_owner, @store_dir, @store_acctid: because EXRPCs APIs are terribly designed
* @ctnt: owned by whatever alloc_context exmdb_client was fed with
*/
struct deleter {
void operator()(message_content *x) const { message_content_free(x); }
};

rxparam(message_node &&in);
NOMOVE(rxparam);
ec_error_t run();
ec_error_t is_oof(bool *out) const;
ec_error_t load_std_rules(bool oof, std::vector<rule_node> &out) const;
ec_error_t load_ext_rules(bool oof, std::vector<rule_node> &out) const;

const char *ev_from = nullptr, *ev_to = nullptr;
message_node cur;
message_content_ptr ctnt;
message_content *ctnt = nullptr;
bool del = false, exit = false, do_autoproc = true;
};

Expand Down Expand Up @@ -824,7 +822,7 @@ static ec_error_t op_process(rxparam &par, const rule_node &rule)
if (rule.cond != nullptr) {
if (g_ruleproc_debug)
mlog(LV_DEBUG, "Rule_Condition %s", rule.cond->repr().c_str());
if (!rx_eval_props(par.ctnt.get(), par.ctnt->proplist, *rule.cond))
if (!rx_eval_props(par.ctnt, par.ctnt->proplist, *rule.cond))
return ecSuccess;
}
if (rule.state & ST_EXIT_LEVEL)
Expand Down Expand Up @@ -860,7 +858,7 @@ static ec_error_t opx_process(rxparam &par, const rule_node &rule)
if (par.exit && !(rule.state & ST_ONLY_WHEN_OOF))
return ecSuccess;
if (rule.cond != nullptr &&
!rx_eval_props(par.ctnt.get(), par.ctnt->proplist, *rule.cond))
!rx_eval_props(par.ctnt, par.ctnt->proplist, *rule.cond))
return ecSuccess;
if (rule.state & ST_EXIT_LEVEL)
par.exit = true;
Expand Down Expand Up @@ -917,7 +915,7 @@ static ec_error_t mr_mark_done(rxparam &par)
uint64_t cal_mid = par.cur.mid, cal_cn = 0;
ec_error_t err = ecSuccess;
if (!exmdb_client->write_message_v2(par.cur.dir.c_str(), CP_ACP,
par.cur.fid, par.ctnt.get(), &cal_mid, &cal_cn, &err))
par.cur.fid, par.ctnt, &cal_mid, &cal_cn, &err))
return ecRpcFailed;
return err;
}
Expand Down Expand Up @@ -1220,7 +1218,7 @@ ec_error_t rxparam::run()
std::sort(rule_list.begin(), rule_list.end());

if (!exmdb_client->read_message(cur.dirc(), nullptr, CP_ACP,
cur.mid, &unique_tie(ctnt)))
cur.mid, &ctnt))
return ecError;
if (ctnt == nullptr)
return ecNotFound;
Expand Down

0 comments on commit ec8e477

Please sign in to comment.