Skip to content

Commit 5aef705

Browse files
committed
ICU-23155: Fix buffer overflow in SSearchTest::offsetTest()
https://unicode-org.atlassian.net/browse/ICU-23155 Use std::string instead. Root cause analysis: In printOffsets, while the pointer `s` is advanced, the remaining size `n` remains unchanged to be a constant 4096, and snprintf falsely thinks the buffer has 4096 bytes left.
1 parent 05454c6 commit 5aef705

File tree

1 file changed

+31
-22
lines changed

1 file changed

+31
-22
lines changed

icu4c/source/test/intltest/ssearch.cpp

Lines changed: 31 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,9 @@
2828
#include "ssearch.h"
2929
#include "xmlparser.h"
3030

31+
#include <iomanip>
32+
#include <iostream>
33+
#include <sstream>
3134
#include <stdio.h> // for snprintf
3235

3336
char testId[100];
@@ -524,40 +527,48 @@ UBool OrderList::matchesAt(int32_t offset, const OrderList &other) const
524527
return true;
525528
}
526529

527-
static char *printOffsets(char *buffer, size_t n, OrderList &list)
530+
static char* printOffsets(std::string &buffer, OrderList &list)
528531
{
529532
int32_t size = list.size();
530-
char *s = buffer;
533+
buffer.clear();
534+
buffer.reserve(size * 10 + 2);
535+
std::stringstream s(buffer);
531536

532537
for(int32_t i = 0; i < size; i += 1) {
533538
const Order *order = list.get(i);
534539

535540
if (i != 0) {
536-
s += snprintf(s, n, ", ");
541+
s << ", ";
537542
}
538543

539-
s += snprintf(s, n, "(%d, %d)", order->lowOffset, order->highOffset);
544+
s << "(" << order->lowOffset << ", " << order->highOffset << ")";
540545
}
541546

542-
return buffer;
547+
return buffer.data();
543548
}
544549

545-
static char *printOrders(char *buffer, size_t n, OrderList &list)
550+
static char* printOrders(std::string &buffer, OrderList &list)
546551
{
547552
int32_t size = list.size();
548-
char *s = buffer;
553+
buffer.clear();
554+
// 10 chars is enough room for 8 hex digits plus ", "
555+
buffer.reserve(size * 10 + 2);
556+
std::stringstream s(buffer);
549557

550558
for(int32_t i = 0; i < size; i += 1) {
551559
const Order *order = list.get(i);
552560

553561
if (i != 0) {
554-
s += snprintf(s, n, ", ");
562+
s << ", ";
555563
}
556564

557-
s += snprintf(s, n, "%8.8X", order->order);
565+
s << std::hex
566+
<< std::uppercase
567+
<< std::setw(8)
568+
<< std::setfill('0')
569+
<< order->order;
558570
}
559-
560-
return buffer;
571+
return buffer.data();
561572
}
562573

563574
void SSearchTest::offsetTest()
@@ -631,9 +642,7 @@ void SSearchTest::offsetTest()
631642
errcheckln(status, "Failed to create collator in offsetTest! - %s", u_errorName(status));
632643
return;
633644
}
634-
char buffer[4096]; // A bit of a hack... just happens to be long enough for all the test cases...
635-
// We could allocate one that's the right size by (CE_count * 10) + 2
636-
// 10 chars is enough room for 8 hex digits plus ", ". 2 extra chars for "[" and "]"
645+
std::string buffer;
637646

638647
col->setAttribute(UCOL_NORMALIZATION_MODE, UCOL_ON, status);
639648

@@ -673,20 +682,20 @@ void SSearchTest::offsetTest()
673682

674683
if (forwardList.compare(backwardList)) {
675684
logln("Works with \"%s\"", test[i]);
676-
logln("Forward offsets: [%s]", printOffsets(buffer, sizeof(buffer), forwardList));
677-
// logln("Backward offsets: [%s]", printOffsets(buffer, sizeof(buffer), backwardList));
685+
logln("Forward offsets: [%s]", printOffsets(buffer, forwardList));
686+
// logln("Backward offsets: [%s]", printOffsets(buffer, backwardList));
678687

679-
logln("Forward CEs: [%s]", printOrders(buffer, sizeof(buffer), forwardList));
680-
// logln("Backward CEs: [%s]", printOrders(buffer, sizeof(buffer), backwardList));
688+
logln("Forward CEs: [%s]", printOrders(buffer, forwardList));
689+
// logln("Backward CEs: [%s]", printOrders(buffer, backwardList));
681690

682691
logln();
683692
} else {
684693
errln("Fails with \"%s\"", test[i]);
685-
infoln("Forward offsets: [%s]", printOffsets(buffer, sizeof(buffer), forwardList));
686-
infoln("Backward offsets: [%s]", printOffsets(buffer, sizeof(buffer), backwardList));
694+
infoln("Forward offsets: [%s]", printOffsets(buffer, forwardList));
695+
infoln("Backward offsets: [%s]", printOffsets(buffer, backwardList));
687696

688-
infoln("Forward CEs: [%s]", printOrders(buffer, sizeof(buffer), forwardList));
689-
infoln("Backward CEs: [%s]", printOrders(buffer, sizeof(buffer), backwardList));
697+
infoln("Forward CEs: [%s]", printOrders(buffer, forwardList));
698+
infoln("Backward CEs: [%s]", printOrders(buffer, backwardList));
690699

691700
infoln();
692701
}

0 commit comments

Comments
 (0)