From f88c95acb61b0a7fb6a2fa17b1a5b892b73c0d50 Mon Sep 17 00:00:00 2001 From: AlexPeshkoff Date: Wed, 28 Oct 2020 20:46:27 +0300 Subject: [PATCH] Fixed CORE-6432: Possible buffer overflow in client library in Attachment::getInfo() call --- src/common/classes/ClumpletReader.cpp | 15 +++++- src/common/classes/ClumpletReader.h | 6 ++- src/remote/merge.cpp | 73 ++++++++++++++++----------- 3 files changed, 62 insertions(+), 32 deletions(-) diff --git a/src/common/classes/ClumpletReader.cpp b/src/common/classes/ClumpletReader.cpp index 14d6d99177..b49f025e1b 100644 --- a/src/common/classes/ClumpletReader.cpp +++ b/src/common/classes/ClumpletReader.cpp @@ -86,7 +86,8 @@ void ClumpletReader::dump() const try { ClumpletDump d(kind, getBuffer(), getBufferLength()); - int t = (kind == SpbStart || kind == UnTagged || kind == WideUnTagged || kind == SpbResponse) ? -1 : d.getBufferTag(); + int t = (kind == SpbStart || kind == UnTagged || kind == WideUnTagged || kind == SpbResponse || kind == InfoResponse) ? + -1 : d.getBufferTag(); gds__log("Tag=%d Offset=%d Length=%d Eof=%d\n", t, getCurOffset(), getBufferLength(), isEof()); for (d.rewind(); !(d.isEof()); d.moveNext()) { @@ -240,6 +241,7 @@ UCHAR ClumpletReader::getBufferTag() const case SpbSendItems: case SpbReceiveItems: case SpbResponse: + case InfoResponse: usage_mistake("buffer is not tagged"); return 0; case SpbAttach: @@ -535,6 +537,16 @@ ClumpletReader::ClumpletType ClumpletReader::getClumpletType(UCHAR tag) const } invalid_structure("unrecognized service response tag", tag); break; + case InfoResponse: + switch (tag) + { + case isc_info_end: + case isc_info_truncated: + case isc_info_data_not_ready: + case isc_info_flag_end: + return SingleTpb; + } + return StringSpb; } invalid_structure("unknown clumplet kind", kind); return SingleTpb; @@ -688,6 +700,7 @@ void ClumpletReader::rewind() case SpbSendItems: case SpbReceiveItems: case SpbResponse: + case InfoResponse: cur_offset = 0; break; default: diff --git a/src/common/classes/ClumpletReader.h b/src/common/classes/ClumpletReader.h index 50aed0856a..b886163111 100644 --- a/src/common/classes/ClumpletReader.h +++ b/src/common/classes/ClumpletReader.h @@ -57,7 +57,8 @@ public: WideUnTagged, SpbSendItems, SpbReceiveItems, - SpbResponse + SpbResponse, + InfoResponse }; struct KindList @@ -130,7 +131,8 @@ public: FB_SIZE_T rc = getBufferEnd() - getBuffer(); if (rc == 1 && kind != UnTagged && kind != SpbStart && kind != WideUnTagged && kind != SpbSendItems && - kind != SpbReceiveItems && kind != SpbResponse) + kind != SpbReceiveItems && kind != SpbResponse && + kind != InfoResponse) { rc = 0; } diff --git a/src/remote/merge.cpp b/src/remote/merge.cpp index 8a06725b11..c9b2093623 100644 --- a/src/remote/merge.cpp +++ b/src/remote/merge.cpp @@ -37,12 +37,12 @@ inline void PUT_WORD(UCHAR*& ptr, USHORT value) #define PUT(ptr, value) *(ptr)++ = value; -static ISC_STATUS merge_setup(const UCHAR**, UCHAR**, const UCHAR* const, USHORT); +static ISC_STATUS merge_setup(const Firebird::ClumpletReader&, UCHAR**, const UCHAR* const, FB_SIZE_T); -USHORT MERGE_database_info(const UCHAR* in, +USHORT MERGE_database_info(const UCHAR* const in, UCHAR* out, - USHORT out_length, + USHORT buf_length, USHORT impl, USHORT class_, USHORT base_level, @@ -65,22 +65,37 @@ USHORT MERGE_database_info(const UCHAR* in, const UCHAR* p; UCHAR* start = out; - const UCHAR* const end = out + out_length; + const UCHAR* const end = out + buf_length; UCHAR mergeLevel = 0; - for (const UCHAR* getMergeLevel = in; - *getMergeLevel != isc_info_end && *getMergeLevel != isc_info_truncated; - getMergeLevel += (3 + gds__vax_integer(getMergeLevel + 1, 2))) + Firebird::ClumpletReader input(Firebird::ClumpletReader::InfoResponse, in, buf_length); + while (!input.isEof()) { - if (*getMergeLevel == isc_info_implementation) + bool flStop = true; + switch(input.getClumpTag()) { - mergeLevel = getMergeLevel[3]; + case isc_info_implementation: + mergeLevel = input.getBytes()[0]; + break; + + case isc_info_end: + case isc_info_truncated: + break; + + default: + flStop = false; break; } + + if (flStop) + break; + input.moveNext(); } - for (;;) - switch (*out++ = *in++) + for (input.rewind(); !input.isEof(); input.moveNext()) + { + *out++ = input.getClumpTag(); + switch (input.getClumpTag()) { case isc_info_end: case isc_info_truncated: @@ -90,7 +105,7 @@ USHORT MERGE_database_info(const UCHAR* in, l = static_cast(strlen((char *) (p = version))); if (l > MAX_UCHAR) l = MAX_UCHAR; - if (merge_setup(&in, &out, end, l + 1)) + if (merge_setup(input, &out, end, l + 1)) return 0; for (*out++ = (UCHAR) l; l; --l) *out++ = *p++; @@ -100,21 +115,21 @@ USHORT MERGE_database_info(const UCHAR* in, l = static_cast(strlen((SCHAR *) (p = id))); if (l > MAX_UCHAR) l = MAX_UCHAR; - if (merge_setup(&in, &out, end, l + 1)) + if (merge_setup(input, &out, end, l + 1)) return 0; for (*out++ = (UCHAR) l; l; --l) *out++ = *p++; break; case isc_info_implementation: - if (merge_setup(&in, &out, end, 2)) + if (merge_setup(input, &out, end, 2)) return 0; PUT(out, (UCHAR) impl); PUT(out, (UCHAR) class_); break; case fb_info_implementation: - if (merge_setup(&in, &out, end, 6)) + if (merge_setup(input, &out, end, 6)) return 0; Firebird::DbImplementation::current.stuff(&out); PUT(out, (UCHAR) class_); @@ -122,30 +137,32 @@ USHORT MERGE_database_info(const UCHAR* in, break; case isc_info_base_level: - if (merge_setup(&in, &out, end, 1)) + if (merge_setup(input, &out, end, 1)) return 0; PUT(out, (UCHAR) base_level); break; default: { - USHORT length = (USHORT) gds__vax_integer(in, 2); - in += 2; + USHORT length = input.getClumpLength(); if (out + length + 2 >= end) { out[-1] = isc_info_truncated; return 0; } PUT_WORD(out, length); - while (length--) - *out++ = *in++; + memcpy(out, input.getBytes(), length); + out += length; } break; } + } + + return 0; // error - missing isc_info_end item } -static ISC_STATUS merge_setup(const UCHAR** in, UCHAR** out, const UCHAR* const end, - USHORT delta_length) +static ISC_STATUS merge_setup(const Firebird::ClumpletReader& input, UCHAR** out, const UCHAR* const end, + FB_SIZE_T delta_length) { /************************************** * @@ -159,17 +176,16 @@ static ISC_STATUS merge_setup(const UCHAR** in, UCHAR** out, const UCHAR* const * already there. * **************************************/ - USHORT length = (USHORT) gds__vax_integer(*in, 2); - const USHORT new_length = length + delta_length; + FB_SIZE_T length = input.getClumpLength(); + const FB_SIZE_T new_length = length + delta_length; - if (*out + new_length + 2 >= end) + if (new_length > MAX_USHORT || *out + new_length + 2 >= end) { (*out)[-1] = isc_info_truncated; return FB_FAILURE; } - *in += 2; - const USHORT count = 1 + *(*in)++; + const USHORT count = 1 + *(input.getBytes()); PUT_WORD(*out, new_length); PUT(*out, (UCHAR) count); @@ -177,9 +193,8 @@ static ISC_STATUS merge_setup(const UCHAR** in, UCHAR** out, const UCHAR* const if (--length) { - memcpy(*out, *in, length); + memcpy(*out, input.getBytes() + 1, length); *out += length; - *in += length; } return FB_SUCCESS;