Skip to content

Commit ea6dc2a

Browse files
AlexPeshkoffAlexPeshkoff
committed
Backported CORE-6432: Possible buffer overflow in client library in Attachment::getInfo() call
1 parent 1d2944c commit ea6dc2a

3 files changed

Lines changed: 62 additions & 32 deletions

File tree

‎src/common/classes/ClumpletReader.cpp‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,8 @@ void ClumpletReader::dump() const
8787

8888
try {
8989
ClumpletDump d(kind, getBuffer(), getBufferLength());
90-
int t = (kind == SpbStart || kind == UnTagged || kind == WideUnTagged || kind == SpbResponse) ? -1 : d.getBufferTag();
90+
int t = (kind == SpbStart || kind == UnTagged || kind == WideUnTagged || kind == SpbResponse || kind == InfoResponse) ?
91+
-1 : d.getBufferTag();
9192
gds__log("Tag=%d Offset=%d Length=%d Eof=%d\n", t, getCurOffset(), getBufferLength(), isEof());
9293
for (d.rewind(); !(d.isEof()); d.moveNext())
9394
{
@@ -241,6 +242,7 @@ UCHAR ClumpletReader::getBufferTag() const
241242
case SpbSendItems:
242243
case SpbReceiveItems:
243244
case SpbResponse:
245+
case InfoResponse:
244246
usage_mistake("buffer is not tagged");
245247
return 0;
246248
case SpbAttach:
@@ -523,6 +525,16 @@ ClumpletReader::ClumpletType ClumpletReader::getClumpletType(UCHAR tag) const
523525
}
524526
invalid_structure("unrecognized service response tag");
525527
break;
528+
case InfoResponse:
529+
switch (tag)
530+
{
531+
case isc_info_end:
532+
case isc_info_truncated:
533+
case isc_info_data_not_ready:
534+
case isc_info_flag_end:
535+
return SingleTpb;
536+
}
537+
return StringSpb;
526538
}
527539
invalid_structure("unknown clumplet kind");
528540
return SingleTpb;
@@ -675,6 +687,7 @@ void ClumpletReader::rewind()
675687
case SpbSendItems:
676688
case SpbReceiveItems:
677689
case SpbResponse:
690+
case InfoResponse:
678691
cur_offset = 0;
679692
break;
680693
default:

‎src/common/classes/ClumpletReader.h‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,8 @@ class ClumpletReader : protected AutoStorage
5959
WideUnTagged,
6060
SpbSendItems,
6161
SpbReceiveItems,
62-
SpbResponse
62+
SpbResponse,
63+
InfoResponse
6364
};
6465

6566
struct KindList
@@ -124,7 +125,8 @@ class ClumpletReader : protected AutoStorage
124125
FB_SIZE_T rc = getBufferEnd() - getBuffer();
125126
if (rc == 1 && kind != UnTagged && kind != SpbStart &&
126127
kind != WideUnTagged && kind != SpbSendItems &&
127-
kind != SpbReceiveItems && kind != SpbResponse)
128+
kind != SpbReceiveItems && kind != SpbResponse &&
129+
kind != InfoResponse)
128130
{
129131
rc = 0;
130132
}

‎src/remote/merge.cpp‎

Lines changed: 44 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -37,12 +37,12 @@ inline void PUT_WORD(UCHAR*& ptr, USHORT value)
3737

3838
#define PUT(ptr, value) *(ptr)++ = value;
3939

40-
static ISC_STATUS merge_setup(const UCHAR**, UCHAR**, const UCHAR* const, USHORT);
40+
static ISC_STATUS merge_setup(const Firebird::ClumpletReader&, UCHAR**, const UCHAR* const, FB_SIZE_T);
4141

4242

43-
USHORT MERGE_database_info(const UCHAR* in,
43+
USHORT MERGE_database_info(const UCHAR* const in,
4444
UCHAR* out,
45-
USHORT out_length,
45+
USHORT buf_length,
4646
USHORT impl,
4747
USHORT class_,
4848
USHORT base_level,
@@ -66,22 +66,37 @@ USHORT MERGE_database_info(const UCHAR* in,
6666
const UCHAR* p;
6767

6868
UCHAR* start = out;
69-
const UCHAR* const end = out + out_length;
69+
const UCHAR* const end = out + buf_length;
7070

7171
UCHAR mergeLevel = 0;
72-
for (const UCHAR* getMergeLevel = in;
73-
*getMergeLevel != isc_info_end && *getMergeLevel != isc_info_truncated;
74-
getMergeLevel += (3 + gds__vax_integer(getMergeLevel + 1, 2)))
72+
Firebird::ClumpletReader input(Firebird::ClumpletReader::InfoResponse, in, buf_length);
73+
while (!input.isEof())
7574
{
76-
if (*getMergeLevel == isc_info_implementation)
75+
bool flStop = true;
76+
switch(input.getClumpTag())
7777
{
78-
mergeLevel = getMergeLevel[3];
78+
case isc_info_implementation:
79+
mergeLevel = input.getBytes()[0];
80+
break;
81+
82+
case isc_info_end:
83+
case isc_info_truncated:
84+
break;
85+
86+
default:
87+
flStop = false;
7988
break;
8089
}
90+
91+
if (flStop)
92+
break;
93+
input.moveNext();
8194
}
8295

83-
for (;;)
84-
switch (*out++ = *in++)
96+
for (input.rewind(); !input.isEof(); input.moveNext())
97+
{
98+
*out++ = input.getClumpTag();
99+
switch (input.getClumpTag())
85100
{
86101
case isc_info_end:
87102
case isc_info_truncated:
@@ -91,7 +106,7 @@ USHORT MERGE_database_info(const UCHAR* in,
91106
l = static_cast<SSHORT>(strlen((char *) (p = version)));
92107
if (l > MAX_UCHAR)
93108
l = MAX_UCHAR;
94-
if (merge_setup(&in, &out, end, l + 1))
109+
if (merge_setup(input, &out, end, l + 1))
95110
return 0;
96111
for (*out++ = (UCHAR) l; l; --l)
97112
*out++ = *p++;
@@ -101,52 +116,54 @@ USHORT MERGE_database_info(const UCHAR* in,
101116
l = static_cast<SSHORT>(strlen((SCHAR *) (p = id)));
102117
if (l > MAX_UCHAR)
103118
l = MAX_UCHAR;
104-
if (merge_setup(&in, &out, end, l + 1))
119+
if (merge_setup(input, &out, end, l + 1))
105120
return 0;
106121
for (*out++ = (UCHAR) l; l; --l)
107122
*out++ = *p++;
108123
break;
109124

110125
case isc_info_implementation:
111-
if (merge_setup(&in, &out, end, 2))
126+
if (merge_setup(input, &out, end, 2))
112127
return 0;
113128
PUT(out, (UCHAR) impl);
114129
PUT(out, (UCHAR) class_);
115130
break;
116131

117132
case fb_info_implementation:
118-
if (merge_setup(&in, &out, end, 6))
133+
if (merge_setup(input, &out, end, 6))
119134
return 0;
120135
Firebird::DbImplementation::current.stuff(&out);
121136
PUT(out, (UCHAR) class_);
122137
PUT(out, mergeLevel);
123138
break;
124139

125140
case isc_info_base_level:
126-
if (merge_setup(&in, &out, end, 1))
141+
if (merge_setup(input, &out, end, 1))
127142
return 0;
128143
PUT(out, (UCHAR) base_level);
129144
break;
130145

131146
default:
132147
{
133-
USHORT length = (USHORT) gds__vax_integer(in, 2);
134-
in += 2;
148+
USHORT length = input.getClumpLength();
135149
if (out + length + 2 >= end)
136150
{
137151
out[-1] = isc_info_truncated;
138152
return 0;
139153
}
140154
PUT_WORD(out, length);
141-
while (length--)
142-
*out++ = *in++;
155+
memcpy(out, input.getBytes(), length);
156+
out += length;
143157
}
144158
break;
145159
}
160+
}
161+
162+
return 0; // error - missing isc_info_end item
146163
}
147164

148-
static ISC_STATUS merge_setup(const UCHAR** in, UCHAR** out, const UCHAR* const end,
149-
USHORT delta_length)
165+
static ISC_STATUS merge_setup(const Firebird::ClumpletReader& input, UCHAR** out, const UCHAR* const end,
166+
FB_SIZE_T delta_length)
150167
{
151168
/**************************************
152169
*
@@ -160,27 +177,25 @@ static ISC_STATUS merge_setup(const UCHAR** in, UCHAR** out, const UCHAR* const
160177
* already there.
161178
*
162179
**************************************/
163-
USHORT length = (USHORT) gds__vax_integer(*in, 2);
164-
const USHORT new_length = length + delta_length;
180+
FB_SIZE_T length = input.getClumpLength();
181+
const FB_SIZE_T new_length = length + delta_length;
165182

166-
if (*out + new_length + 2 >= end)
183+
if (new_length > MAX_USHORT || *out + new_length + 2 >= end)
167184
{
168185
(*out)[-1] = isc_info_truncated;
169186
return FB_FAILURE;
170187
}
171188

172-
*in += 2;
173-
const USHORT count = 1 + *(*in)++;
189+
const USHORT count = 1 + *(input.getBytes());
174190
PUT_WORD(*out, new_length);
175191
PUT(*out, (UCHAR) count);
176192

177193
// Copy data portion of information sans original count
178194

179195
if (--length)
180196
{
181-
memcpy(*out, *in, length);
197+
memcpy(*out, input.getBytes() + 1, length);
182198
*out += length;
183-
*in += length;
184199
}
185200

186201
return FB_SUCCESS;

0 commit comments

Comments
 (0)