Skip to content

Commit f88c95a

Browse files
AlexPeshkoffAlexPeshkoff
committed
Fixed CORE-6432: Possible buffer overflow in client library in Attachment::getInfo() call
1 parent ca77a37 commit f88c95a

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
@@ -86,7 +86,8 @@ void ClumpletReader::dump() const
8686

8787
try {
8888
ClumpletDump d(kind, getBuffer(), getBufferLength());
89-
int t = (kind == SpbStart || kind == UnTagged || kind == WideUnTagged || kind == SpbResponse) ? -1 : d.getBufferTag();
89+
int t = (kind == SpbStart || kind == UnTagged || kind == WideUnTagged || kind == SpbResponse || kind == InfoResponse) ?
90+
-1 : d.getBufferTag();
9091
gds__log("Tag=%d Offset=%d Length=%d Eof=%d\n", t, getCurOffset(), getBufferLength(), isEof());
9192
for (d.rewind(); !(d.isEof()); d.moveNext())
9293
{
@@ -240,6 +241,7 @@ UCHAR ClumpletReader::getBufferTag() const
240241
case SpbSendItems:
241242
case SpbReceiveItems:
242243
case SpbResponse:
244+
case InfoResponse:
243245
usage_mistake("buffer is not tagged");
244246
return 0;
245247
case SpbAttach:
@@ -535,6 +537,16 @@ ClumpletReader::ClumpletType ClumpletReader::getClumpletType(UCHAR tag) const
535537
}
536538
invalid_structure("unrecognized service response tag", tag);
537539
break;
540+
case InfoResponse:
541+
switch (tag)
542+
{
543+
case isc_info_end:
544+
case isc_info_truncated:
545+
case isc_info_data_not_ready:
546+
case isc_info_flag_end:
547+
return SingleTpb;
548+
}
549+
return StringSpb;
538550
}
539551
invalid_structure("unknown clumplet kind", kind);
540552
return SingleTpb;
@@ -688,6 +700,7 @@ void ClumpletReader::rewind()
688700
case SpbSendItems:
689701
case SpbReceiveItems:
690702
case SpbResponse:
703+
case InfoResponse:
691704
cur_offset = 0;
692705
break;
693706
default:

‎src/common/classes/ClumpletReader.h‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,8 @@ class ClumpletReader : protected AutoStorage
5757
WideUnTagged,
5858
SpbSendItems,
5959
SpbReceiveItems,
60-
SpbResponse
60+
SpbResponse,
61+
InfoResponse
6162
};
6263

6364
struct KindList
@@ -130,7 +131,8 @@ class ClumpletReader : protected AutoStorage
130131
FB_SIZE_T rc = getBufferEnd() - getBuffer();
131132
if (rc == 1 && kind != UnTagged && kind != SpbStart &&
132133
kind != WideUnTagged && kind != SpbSendItems &&
133-
kind != SpbReceiveItems && kind != SpbResponse)
134+
kind != SpbReceiveItems && kind != SpbResponse &&
135+
kind != InfoResponse)
134136
{
135137
rc = 0;
136138
}

‎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,
@@ -65,22 +65,37 @@ USHORT MERGE_database_info(const UCHAR* in,
6565
const UCHAR* p;
6666

6767
UCHAR* start = out;
68-
const UCHAR* const end = out + out_length;
68+
const UCHAR* const end = out + buf_length;
6969

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

82-
for (;;)
83-
switch (*out++ = *in++)
95+
for (input.rewind(); !input.isEof(); input.moveNext())
96+
{
97+
*out++ = input.getClumpTag();
98+
switch (input.getClumpTag())
8499
{
85100
case isc_info_end:
86101
case isc_info_truncated:
@@ -90,7 +105,7 @@ USHORT MERGE_database_info(const UCHAR* in,
90105
l = static_cast<SSHORT>(strlen((char *) (p = version)));
91106
if (l > MAX_UCHAR)
92107
l = MAX_UCHAR;
93-
if (merge_setup(&in, &out, end, l + 1))
108+
if (merge_setup(input, &out, end, l + 1))
94109
return 0;
95110
for (*out++ = (UCHAR) l; l; --l)
96111
*out++ = *p++;
@@ -100,52 +115,54 @@ USHORT MERGE_database_info(const UCHAR* in,
100115
l = static_cast<SSHORT>(strlen((SCHAR *) (p = id)));
101116
if (l > MAX_UCHAR)
102117
l = MAX_UCHAR;
103-
if (merge_setup(&in, &out, end, l + 1))
118+
if (merge_setup(input, &out, end, l + 1))
104119
return 0;
105120
for (*out++ = (UCHAR) l; l; --l)
106121
*out++ = *p++;
107122
break;
108123

109124
case isc_info_implementation:
110-
if (merge_setup(&in, &out, end, 2))
125+
if (merge_setup(input, &out, end, 2))
111126
return 0;
112127
PUT(out, (UCHAR) impl);
113128
PUT(out, (UCHAR) class_);
114129
break;
115130

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

124139
case isc_info_base_level:
125-
if (merge_setup(&in, &out, end, 1))
140+
if (merge_setup(input, &out, end, 1))
126141
return 0;
127142
PUT(out, (UCHAR) base_level);
128143
break;
129144

130145
default:
131146
{
132-
USHORT length = (USHORT) gds__vax_integer(in, 2);
133-
in += 2;
147+
USHORT length = input.getClumpLength();
134148
if (out + length + 2 >= end)
135149
{
136150
out[-1] = isc_info_truncated;
137151
return 0;
138152
}
139153
PUT_WORD(out, length);
140-
while (length--)
141-
*out++ = *in++;
154+
memcpy(out, input.getBytes(), length);
155+
out += length;
142156
}
143157
break;
144158
}
159+
}
160+
161+
return 0; // error - missing isc_info_end item
145162
}
146163

147-
static ISC_STATUS merge_setup(const UCHAR** in, UCHAR** out, const UCHAR* const end,
148-
USHORT delta_length)
164+
static ISC_STATUS merge_setup(const Firebird::ClumpletReader& input, UCHAR** out, const UCHAR* const end,
165+
FB_SIZE_T delta_length)
149166
{
150167
/**************************************
151168
*
@@ -159,27 +176,25 @@ static ISC_STATUS merge_setup(const UCHAR** in, UCHAR** out, const UCHAR* const
159176
* already there.
160177
*
161178
**************************************/
162-
USHORT length = (USHORT) gds__vax_integer(*in, 2);
163-
const USHORT new_length = length + delta_length;
179+
FB_SIZE_T length = input.getClumpLength();
180+
const FB_SIZE_T new_length = length + delta_length;
164181

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

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

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

178194
if (--length)
179195
{
180-
memcpy(*out, *in, length);
196+
memcpy(*out, input.getBytes() + 1, length);
181197
*out += length;
182-
*in += length;
183198
}
184199

185200
return FB_SUCCESS;

0 commit comments

Comments
 (0)