Skip to content

Commit 41cd1a7

Browse files
jengelhjengelh
committed
mapi_lib: fix out-of-bounds access in PROBLEM_ARRAY::transform
Triggerable upon performing an online-mode search in OL2019. ==93973==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x502000087734 at pc 0x7ff932e5635f bp 0x7ff8a79fd240 sp 0x7ff8a79fd238 READ of size 2 at 0x502000087734 thread T73 f0 PROBLEM_ARRAY::transform(unsigned short const*) lib/mapi/element_data.cpp:407 f1 folder_object::set_properties(TPROPVAL_ARRAY const*, PROBLEM_ARRAY*) exch/emsmdb/folder_object.cpp:522 f2 rop_setproperties(TPROPVAL_ARRAY const*, PROBLEM_ARRAY*, LOGMAP*, unsigned char, unsigned int) exch/emsmdb/oxcprpt.cpp:385 f3 rop_dispatch(rop_request const&, rop_response*&, unsigned int*, unsigned char) exch/emsmdb/rop_dispatch.cpp:1062 0x502000087734 is located 2 bytes after 2-byte region [0x502000087730,0x502000087732) allocated by thread T73 here: f0 operator new[](unsigned long) (/lib64/libasan.so.8) f1 std::__detail::_MakeUniq<char []>::__array std::make_unique<char []>(unsigned long) /usr/include/c++/13/bits/unique_ptr.h:1085 f2 alloc_context::alloc(unsigned long) include/gromox/util.hpp:39 f3 pdu_processor_ndr_stack_alloc(int, unsigned long) exch/http/pdu_processor.cpp:162 f4 emsmdb::common_util_alloc(unsigned long) exch/emsmdb/common_util.cpp:88 f5 unsigned short* emsmdb::cu_alloc<unsigned short>(unsigned long) exch/emsmdb/common_util.hpp:37 f6 folder_object::set_properties(TPROPVAL_ARRAY const*, PROBLEM_ARRAY*) exch/emsmdb/folder_object.cpp:482 f7 rop_setproperties(TPROPVAL_ARRAY const*, PROBLEM_ARRAY*, LOGMAP*, unsigned char, unsigned int) exch/emsmdb/oxcprpt.cpp:385 f8 rop_dispatch(rop_request const&, rop_response*&, unsigned int*, unsigned char) exch/emsmdb/rop_dispatch.cpp:1062 According to this report, poriginal_indices must have been a 1-element array and tmp_propvals 5-element, of which the 3rd, PR_CHANGE_KEY, (somewhat mysteriously) was reported in PROBLEM_ARRAY. Fixes: gromox-0~666 References: GXL-503, DESK-2209
1 parent c39628c commit 41cd1a7

File tree

8 files changed

+89
-58
lines changed

8 files changed

+89
-58
lines changed

‎exch/emsmdb/attachment_object.cpp‎

Lines changed: 18 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,13 @@
11
// SPDX-License-Identifier: GPL-2.0-only WITH linking exception
2+
// SPDX-FileCopyrightText: 2021–2024 grommunio GmbH
3+
// This file is part of Gromox.
24
#include <climits>
35
#include <cstdint>
46
#include <cstdlib>
57
#include <cstring>
68
#include <memory>
79
#include <utility>
10+
#include <vector>
811
#include <gromox/defs.h>
912
#include <gromox/mapidefs.h>
1013
#include <gromox/proptag_array.hpp>
@@ -16,6 +19,8 @@
1619
#include "message_object.hpp"
1720
#include "stream_object.hpp"
1821

22+
using namespace gromox;
23+
1924
static constexpr uint32_t indet_rendering_pos = UINT32_MAX;
2025

2126
std::unique_ptr<attachment_object> attachment_object::create(message_object *pparent,
@@ -316,7 +321,7 @@ static bool ao_has_open_streams(attachment_object *at, uint32_t proptag)
316321
}
317322

318323
BOOL attachment_object::set_properties(const TPROPVAL_ARRAY *ppropvals,
319-
PROBLEM_ARRAY *pproblems)
324+
PROBLEM_ARRAY *pproblems) try
320325
{
321326
auto pattachment = this;
322327
PROBLEM_ARRAY tmp_problems;
@@ -330,18 +335,16 @@ BOOL attachment_object::set_properties(const TPROPVAL_ARRAY *ppropvals,
330335
tmp_propvals.ppropval = cu_alloc<TAGGED_PROPVAL>(ppropvals->count);
331336
if (tmp_propvals.ppropval == nullptr)
332337
return FALSE;
333-
auto poriginal_indices = cu_alloc<uint16_t>(ppropvals->count);
334-
if (poriginal_indices == nullptr)
335-
return FALSE;
338+
std::vector<uint16_t> poriginal_indices;
336339
for (unsigned int i = 0; i < ppropvals->count; ++i) {
337340
const auto &pv = ppropvals->ppropval[i];
338341
if (is_readonly_prop(pv.proptag) ||
339342
ao_has_open_streams(pattachment, pv.proptag)) {
340343
pproblems->emplace_back(i, pv.proptag, ecAccessDenied);
341344
continue;
342345
}
343-
tmp_propvals.ppropval[tmp_propvals.count] = pv;
344-
poriginal_indices[tmp_propvals.count++] = i;
346+
tmp_propvals.ppropval[tmp_propvals.count++] = pv;
347+
poriginal_indices.push_back(i);
345348
}
346349
if (tmp_propvals.count == 0)
347350
return TRUE;
@@ -361,10 +364,13 @@ BOOL attachment_object::set_properties(const TPROPVAL_ARRAY *ppropvals,
361364
}
362365
}
363366
return TRUE;
367+
} catch (const std::bad_alloc &) {
368+
mlog(LV_ERR, "E-1669: ENOMEM");
369+
return false;
364370
}
365371

366372
BOOL attachment_object::remove_properties(const PROPTAG_ARRAY *pproptags,
367-
PROBLEM_ARRAY *pproblems)
373+
PROBLEM_ARRAY *pproblems) try
368374
{
369375
auto pattachment = this;
370376
PROBLEM_ARRAY tmp_problems;
@@ -378,17 +384,15 @@ BOOL attachment_object::remove_properties(const PROPTAG_ARRAY *pproptags,
378384
tmp_proptags.pproptag = cu_alloc<uint32_t>(pproptags->count);
379385
if (tmp_proptags.pproptag == nullptr)
380386
return FALSE;
381-
auto poriginal_indices = cu_alloc<uint16_t>(pproptags->count);
382-
if (poriginal_indices == nullptr)
383-
return FALSE;
387+
std::vector<uint16_t> poriginal_indices;
384388
for (unsigned int i = 0; i < pproptags->count; ++i) {
385389
const auto tag = pproptags->pproptag[i];
386390
if (is_readonly_prop(tag) ||
387391
ao_has_open_streams(pattachment, tag)) {
388392
pproblems->emplace_back(i, tag, ecAccessDenied);
389393
continue;
390394
}
391-
poriginal_indices[tmp_proptags.count] = i;
395+
poriginal_indices.push_back(i);
392396
tmp_proptags.emplace_back(tag);
393397
}
394398
if (tmp_proptags.count == 0)
@@ -409,6 +413,9 @@ BOOL attachment_object::remove_properties(const PROPTAG_ARRAY *pproptags,
409413
}
410414
}
411415
return TRUE;
416+
} catch (const std::bad_alloc &) {
417+
mlog(LV_ERR, "E-1670: ENOMEM");
418+
return false;
412419
}
413420

414421
BOOL attachment_object::copy_properties(attachment_object *pattachment_src,

‎exch/emsmdb/folder_object.cpp‎

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
11
// SPDX-License-Identifier: GPL-2.0-only WITH linking exception
2+
// SPDX-FileCopyrightText: 2021–2024 grommunio GmbH
3+
// This file is part of Gromox.
24
#include <algorithm>
35
#include <climits>
46
#include <cstdint>
@@ -7,6 +9,7 @@
79
#include <cstring>
810
#include <memory>
911
#include <utility>
12+
#include <vector>
1013
#include <gromox/defs.h>
1114
#include <gromox/ext_buffer.hpp>
1215
#include <gromox/mapidefs.h>
@@ -18,6 +21,8 @@
1821
#include "folder_object.hpp"
1922
#include "logon_object.hpp"
2023

24+
using namespace gromox;
25+
2126
std::unique_ptr<folder_object> folder_object::create(logon_object *plogon,
2227
uint64_t folder_id, uint8_t type, uint32_t tag_access)
2328
{
@@ -458,7 +463,7 @@ BOOL folder_object::get_properties(const PROPTAG_ARRAY *pproptags,
458463
}
459464

460465
BOOL folder_object::set_properties(const TPROPVAL_ARRAY *ppropvals,
461-
PROBLEM_ARRAY *pproblems)
466+
PROBLEM_ARRAY *pproblems) try
462467
{
463468
uint16_t count;
464469
BINARY *pbin_pcl;
@@ -479,17 +484,15 @@ BOOL folder_object::set_properties(const TPROPVAL_ARRAY *ppropvals,
479484
tmp_propvals.ppropval = cu_alloc<TAGGED_PROPVAL>(count);
480485
if (tmp_propvals.ppropval == nullptr)
481486
return FALSE;
482-
auto poriginal_indices = cu_alloc<uint16_t>(ppropvals->count);
483-
if (poriginal_indices == nullptr)
484-
return FALSE;
487+
std::vector<uint16_t> poriginal_indices;
485488
auto pfolder = this;
486489
for (unsigned int i = 0; i < ppropvals->count; ++i) {
487490
const auto &pv = ppropvals->ppropval[i];
488491
if (pfolder->is_readonly_prop(pv.proptag)) {
489492
pproblems->emplace_back(i, pv.proptag, ecAccessDenied);
490493
} else {
491-
tmp_propvals.ppropval[tmp_propvals.count] = pv;
492-
poriginal_indices[tmp_propvals.count++] = i;
494+
tmp_propvals.ppropval[tmp_propvals.count++] = pv;
495+
poriginal_indices.push_back(i);
493496
}
494497
}
495498
if (tmp_propvals.count == 0)
@@ -522,6 +525,9 @@ BOOL folder_object::set_properties(const TPROPVAL_ARRAY *ppropvals,
522525
tmp_problems.transform(poriginal_indices);
523526
*pproblems += std::move(tmp_problems);
524527
return TRUE;
528+
} catch (const std::bad_alloc &) {
529+
mlog(LV_ERR, "E-1743: ENOMEM");
530+
return false;
525531
}
526532

527533
BOOL folder_object::remove_properties(const PROPTAG_ARRAY *pproptags,

‎exch/emsmdb/logon_object.cpp‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
#include <cstring>
1111
#include <memory>
1212
#include <utility>
13+
#include <vector>
1314
#include <libHX/string.h>
1415
#include <gromox/defs.h>
1516
#include <gromox/mapidefs.h>
@@ -664,7 +665,7 @@ BOOL logon_object::get_properties(const PROPTAG_ARRAY *pproptags,
664665
}
665666

666667
BOOL logon_object::set_properties(const TPROPVAL_ARRAY *ppropvals,
667-
PROBLEM_ARRAY *pproblems)
668+
PROBLEM_ARRAY *pproblems) try
668669
{
669670
PROBLEM_ARRAY tmp_problems;
670671
TPROPVAL_ARRAY tmp_propvals;
@@ -680,17 +681,15 @@ BOOL logon_object::set_properties(const TPROPVAL_ARRAY *ppropvals,
680681
tmp_propvals.ppropval = cu_alloc<TAGGED_PROPVAL>(ppropvals->count);
681682
if (tmp_propvals.ppropval == nullptr)
682683
return FALSE;
683-
auto poriginal_indices = cu_alloc<uint16_t>(ppropvals->count);
684-
if (poriginal_indices == nullptr)
685-
return FALSE;
684+
std::vector<uint16_t> poriginal_indices;
686685
auto plogon = this;
687686
for (unsigned int i = 0; i < ppropvals->count; ++i) {
688687
const auto &pv = ppropvals->ppropval[i];
689688
if (lo_is_readonly_prop(plogon, pv.proptag)) {
690689
pproblems->emplace_back(i, pv.proptag, ecAccessDenied);
691690
} else {
692-
tmp_propvals.ppropval[tmp_propvals.count] = pv;
693-
poriginal_indices[tmp_propvals.count++] = i;
691+
tmp_propvals.ppropval[tmp_propvals.count++] = pv;
692+
poriginal_indices.push_back(i);
694693
}
695694
}
696695
if (tmp_propvals.count == 0)
@@ -703,6 +702,9 @@ BOOL logon_object::set_properties(const TPROPVAL_ARRAY *ppropvals,
703702
tmp_problems.transform(poriginal_indices);
704703
*pproblems += std::move(tmp_problems);
705704
return TRUE;
705+
} catch (const std::bad_alloc &) {
706+
mlog(LV_ERR, "E-1744: ENOMEM");
707+
return false;
706708
}
707709

708710
BOOL logon_object::remove_properties(const PROPTAG_ARRAY *pproptags,

‎exch/emsmdb/message_object.cpp‎

Lines changed: 15 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
#include <cstdlib>
77
#include <cstring>
88
#include <memory>
9+
#include <vector>
910
#include <libHX/string.h>
1011
#include <gromox/defs.h>
1112
#include <gromox/ext_buffer.hpp>
@@ -1054,7 +1055,7 @@ static bool mo_has_open_streams(message_object *pmessage, uint32_t proptag)
10541055
}
10551056

10561057
static BOOL message_object_set_properties_internal(message_object *pmessage,
1057-
BOOL b_check, const TPROPVAL_ARRAY *ppropvals, PROBLEM_ARRAY *pproblems)
1058+
BOOL b_check, const TPROPVAL_ARRAY *ppropvals, PROBLEM_ARRAY *pproblems) try
10581059
{
10591060
uint8_t tmp_bytes[3];
10601061
PROBLEM_ARRAY tmp_problems;
@@ -1072,10 +1073,8 @@ static BOOL message_object_set_properties_internal(message_object *pmessage,
10721073
tmp_propvals.ppropval = cu_alloc<TAGGED_PROPVAL>(ppropvals->count);
10731074
if (tmp_propvals.ppropval == nullptr)
10741075
return FALSE;
1075-
auto poriginal_indices = cu_alloc<uint16_t>(ppropvals->count);
1076-
if (poriginal_indices == nullptr)
1077-
return FALSE;
1078-
1076+
1077+
std::vector<uint16_t> poriginal_indices;
10791078
auto dir = pmessage->plogon->get_dir();
10801079
for (unsigned int i = 0; i < ppropvals->count; ++i) {
10811080
/* if property is being open as stream object, can not be modified */
@@ -1116,8 +1115,8 @@ static BOOL message_object_set_properties_internal(message_object *pmessage,
11161115
return FALSE;
11171116
}
11181117
}
1119-
tmp_propvals.ppropval[tmp_propvals.count] = pv;
1120-
poriginal_indices[tmp_propvals.count++] = i;
1118+
tmp_propvals.ppropval[tmp_propvals.count++] = pv;
1119+
poriginal_indices.push_back(i);
11211120
}
11221121
if (tmp_propvals.count == 0)
11231122
return TRUE;
@@ -1142,6 +1141,9 @@ static BOOL message_object_set_properties_internal(message_object *pmessage,
11421141
return FALSE;
11431142
}
11441143
return TRUE;
1144+
} catch (const std::bad_alloc &) {
1145+
mlog(LV_ERR, "E-1745: ENOMEM");
1146+
return false;
11451147
}
11461148

11471149
BOOL message_object::set_properties(const TPROPVAL_ARRAY *ppropvals,
@@ -1153,7 +1155,7 @@ BOOL message_object::set_properties(const TPROPVAL_ARRAY *ppropvals,
11531155
}
11541156

11551157
BOOL message_object::remove_properties(const PROPTAG_ARRAY *pproptags,
1156-
PROBLEM_ARRAY *pproblems)
1158+
PROBLEM_ARRAY *pproblems) try
11571159
{
11581160
auto pmessage = this;
11591161
PROBLEM_ARRAY tmp_problems;
@@ -1169,9 +1171,7 @@ BOOL message_object::remove_properties(const PROPTAG_ARRAY *pproptags,
11691171
tmp_proptags.pproptag = cu_alloc<uint32_t>(pproptags->count);
11701172
if (tmp_proptags.pproptag == nullptr)
11711173
return FALSE;
1172-
auto poriginal_indices = cu_alloc<uint16_t>(pproptags->count);
1173-
if (poriginal_indices == nullptr)
1174-
return FALSE;
1174+
std::vector<uint16_t> poriginal_indices;
11751175
/* if property is being open as stream object, can not be removed */
11761176
for (unsigned int i = 0; i < pproptags->count; ++i) {
11771177
const auto tag = pproptags->pproptag[i];
@@ -1190,7 +1190,7 @@ BOOL message_object::remove_properties(const PROPTAG_ARRAY *pproptags,
11901190
tag, static_cast<unsigned long long>(message_id), instance_id);
11911191
continue;
11921192
default:
1193-
poriginal_indices[tmp_proptags.count] = i;
1193+
poriginal_indices.push_back(i);
11941194
tmp_proptags.emplace_back(tag);
11951195
break;
11961196
}
@@ -1218,6 +1218,9 @@ BOOL message_object::remove_properties(const PROPTAG_ARRAY *pproptags,
12181218
return FALSE;
12191219
}
12201220
return TRUE;
1221+
} catch (const std::bad_alloc &) {
1222+
mlog(LV_ERR, "E-1746: ENOMEM");
1223+
return false;
12211224
}
12221225

12231226
BOOL message_object::copy_to(message_object *pmessage_src,

‎exch/emsmdb/oxcprpt.cpp‎

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
#include <cstdint>
66
#include <cstring>
77
#include <utility>
8+
#include <vector>
89
#include <gromox/defs.h>
910
#include <gromox/mapidefs.h>
1011
#include <gromox/proc_common.h>
@@ -565,7 +566,7 @@ ec_error_t rop_querynamedproperties(uint8_t query_flags, const GUID *pguid,
565566

566567
ec_error_t rop_copyproperties(uint8_t want_asynchronous, uint8_t copy_flags,
567568
const PROPTAG_ARRAY *pproptags, PROBLEM_ARRAY *pproblems, LOGMAP *plogmap,
568-
uint8_t logon_id, uint32_t hsrc, uint32_t hdst)
569+
uint8_t logon_id, uint32_t hsrc, uint32_t hdst) try
569570
{
570571
BOOL b_force;
571572
BOOL b_result;
@@ -600,9 +601,7 @@ ec_error_t rop_copyproperties(uint8_t want_asynchronous, uint8_t copy_flags,
600601
pproblems->pproblem = cu_alloc<PROPERTY_PROBLEM>(pproptags->count);
601602
if (pproblems->pproblem == nullptr)
602603
return ecServerOOM;
603-
auto poriginal_indices = cu_alloc<uint16_t>(pproptags->count);
604-
if (poriginal_indices == nullptr)
605-
return ecError;
604+
std::vector<uint16_t> poriginal_indices;
606605
switch (object_type) {
607606
case ems_objtype::folder: {
608607
auto fldsrc = static_cast<folder_object *>(pobject);
@@ -626,7 +625,7 @@ ec_error_t rop_copyproperties(uint8_t want_asynchronous, uint8_t copy_flags,
626625
}
627626
if (copy_flags & MAPI_NOREPLACE && proptags1.has(tag))
628627
continue;
629-
poriginal_indices[proptags.count] = i;
628+
poriginal_indices.push_back(i);
630629
proptags.emplace_back(tag);
631630
}
632631
if (!fldsrc->get_properties(&proptags, &propvals))
@@ -676,7 +675,7 @@ ec_error_t rop_copyproperties(uint8_t want_asynchronous, uint8_t copy_flags,
676675
}
677676
if (copy_flags & MAPI_NOREPLACE && proptags1.has(tag))
678677
continue;
679-
poriginal_indices[proptags.count] = i;
678+
poriginal_indices.push_back(i);
680679
proptags.emplace_back(tag);
681680
}
682681
if (!msgsrc->get_properties(0, &proptags, &propvals))
@@ -710,7 +709,7 @@ ec_error_t rop_copyproperties(uint8_t want_asynchronous, uint8_t copy_flags,
710709
}
711710
if (copy_flags & MAPI_NOREPLACE && proptags1.has(tag))
712711
continue;
713-
poriginal_indices[proptags.count] = i;
712+
poriginal_indices.push_back(i);
714713
proptags.emplace_back(tag);
715714
}
716715
if (!atsrc->get_properties(0, &proptags, &propvals))
@@ -730,6 +729,9 @@ ec_error_t rop_copyproperties(uint8_t want_asynchronous, uint8_t copy_flags,
730729
default:
731730
return ecNotSupported;
732731
}
732+
} catch (const std::bad_alloc &) {
733+
mlog(LV_ERR, "E-1747: ENOMEM");
734+
return ecServerOOM;
733735
}
734736

735737
ec_error_t rop_copyto(uint8_t want_asynchronous, uint8_t want_subobjects,

0 commit comments

Comments
 (0)