From 54290371797bfd39fb2cb51f4e178ba36a1735d1 Mon Sep 17 00:00:00 2001 From: Tim Eves Date: Thu, 17 Jan 2019 17:12:42 +0700 Subject: [PATCH] Fix bug #1494 potential for buffer overrun The get/put routines now require the buffer size remaining to be read/written to permit checking and the iterator will update this value whenever iterator comparisions are made. Other direct uses of get/put have been updated. --- .../src/km_kbp_context_api.cpp | 12 ++-- .../engine/keyboardprocessor/src/utfcodec.hpp | 59 ++++++++++--------- .../tests/unit/kmnkbd/context_api.cpp | 10 +++- 3 files changed, 44 insertions(+), 37 deletions(-) diff --git a/common/engine/keyboardprocessor/src/km_kbp_context_api.cpp b/common/engine/keyboardprocessor/src/km_kbp_context_api.cpp index 23915acf0b..7a44e13b23 100644 --- a/common/engine/keyboardprocessor/src/km_kbp_context_api.cpp +++ b/common/engine/keyboardprocessor/src/km_kbp_context_api.cpp @@ -32,7 +32,7 @@ namespace { { std::vector res; - for (auto i = typename U::const_sentinal_iterator(text); i; ++i) + for (auto i = typename U::const_iterator(text); *i; ++i) { if(i.error()) return KM_KBP_STATUS_INVALID_UTF; res.emplace_back(km_kbp_context_item {KM_KBP_CT_CHAR, {0,}, {*i}}); @@ -65,7 +65,7 @@ namespace { auto i = typename U::iterator(buf); auto const e = decltype(i)(buf + buf_size - 1); - for (;i != e && ci->type != KM_KBP_CT_END; ++ci) + for (;i != e && ci->type != KM_KBP_CT_END && !i.error(); ++ci) { if (ci->type == KM_KBP_CT_CHAR) { @@ -83,13 +83,13 @@ namespace { else { auto n = 0; - typename U::codeunit_t cps[4/sizeof(typename U::codeunit_t)]; + typename U::codeunit_t cps[4]; do { if (ci->type == KM_KBP_CT_CHAR) { - int8_t l; + int8_t l = 4; U::codec::put(cps, ci->character, l); n += l; } @@ -242,8 +242,8 @@ json & operator << (json & j, km::kbp::context const & ctxt) { json & operator << (json & j, km_kbp_context_item const & i) { - utf8::codeunit_t cps[5] = {0,}; - int8_t l; + utf8::codeunit_t cps[4] = {0,}; + int8_t l = 4; switch (i.type) { diff --git a/common/engine/keyboardprocessor/src/utfcodec.hpp b/common/engine/keyboardprocessor/src/utfcodec.hpp index 3f22288149..3c5eb540dd 100644 --- a/common/engine/keyboardprocessor/src/utfcodec.hpp +++ b/common/engine/keyboardprocessor/src/utfcodec.hpp @@ -11,6 +11,7 @@ */ #pragma once +#include #include #include #include @@ -75,9 +76,11 @@ public: if (usv < 0x10000) { l = 1; cp[0] = codeunit_t(usv); } else { - cp[0] = codeunit_t(lead_offset + (usv >> 10)); - cp[1] = codeunit_t(0xDC00 + (usv & 0x3FF)); - l = 2; + l -= 2; if (l < 0) return; + // We have space so continue. + cp[0] = codeunit_t(lead_offset + (usv >> 10)); + cp[1] = codeunit_t(0xDC00 + (usv & 0x3FF)); + l = 2; } } @@ -85,13 +88,12 @@ public: static uchar_t get(const codeunit_t * cp, int8_t & l) throw() { const uint32_t uh = cp[0]; - l = 1; - if (uh < 0xD800|| uh > 0xDFFF) { return uh; } - if (uh > 0xDBFF) { l = -1; return 0xFFFD; } + if (uh < 0xD800|| uh > 0xDFFF) { l = 1; return uh; } + if (uh > 0xDBFF || l < 2) { l = -1; return 0xFFFD; } const uint32_t ul = cp[1]; if (ul < 0xDC00 || ul > 0xDFFF) { l = -1; return 0xFFFD; } - ++l; + l = 2; return (uh<<10) + ul + surrogate_offset; } @@ -126,10 +128,10 @@ public: inline static void put(codeunit_t * cp, const uchar_t usv, int8_t & l) throw() { - if (usv < 0x80) {l = 1; cp[0] = usv; return; } - if (usv < 0x0800) {l = 2; cp[0] = 0xC0 + (usv >> 6); cp[1] = 0x80 + (usv & 0x3F); return; } - if (usv < 0x10000) {l = 3; cp[0] = 0xE0 + (usv >> 12); cp[1] = 0x80 + ((usv >> 6) & 0x3F); cp[2] = 0x80 + (usv & 0x3F); return; } - else {l = 4; cp[0] = 0xF0 + (usv >> 18); cp[1] = 0x80 + ((usv >> 12) & 0x3F); cp[2] = 0x80 + ((usv >> 6) & 0x3F); cp[3] = 0x80 + (usv & 0x3F); return; } + --l; if (usv < 0x80) {l = 1; cp[0] = usv; return; } + --l; if (usv < 0x0800) {if (l < 0) return; l = 2; cp[0] = 0xC0 + (usv >> 6); cp[1] = 0x80 + (usv & 0x3F); return; }; + --l; if (usv < 0x10000) {if (l < 0) return; l = 3; cp[0] = 0xE0 + (usv >> 12); cp[1] = 0x80 + ((usv >> 6) & 0x3F); cp[2] = 0x80 + (usv & 0x3F); return; } + else {if (--l < 0) return; l = 4; cp[0] = 0xF0 + (usv >> 18); cp[1] = 0x80 + ((usv >> 12) & 0x3F); cp[2] = 0x80 + ((usv >> 6) & 0x3F); cp[3] = 0x80 + (usv & 0x3F); return; }; } inline @@ -137,20 +139,21 @@ public: { const int8_t seq_sz = sz_lut[*cp >> 4]; uchar_t u = *cp & mask_lut[seq_sz]; - l = 1; - bool toolong = false; + if (l < seq_sz) { l = -l; return 0xFFFD; } + auto rl = 1; + bool toolong = false; switch(seq_sz) { - case 4: u <<= 6; u |= *++cp & 0x3F; if (*cp >> 6 != 2) break; ++l; toolong = (u < 0x10); fallthrough; - case 3: u <<= 6; u |= *++cp & 0x3F; if (*cp >> 6 != 2) break; ++l; toolong |= (u < 0x20); fallthrough; - case 2: u <<= 6; u |= *++cp & 0x3F; if (*cp >> 6 != 2) break; ++l; toolong |= (u < 0x80); fallthrough; - case 1: break; + case 4: u <<= 6; u |= *++cp & 0x3F; if (*cp >> 6 != 2) break; ++rl; toolong = (u < 0x10); fallthrough; + case 3: u <<= 6; u |= *++cp & 0x3F; if (*cp >> 6 != 2) break; ++rl; toolong |= (u < 0x20); fallthrough; + case 2: u <<= 6; u |= *++cp & 0x3F; if (*cp >> 6 != 2) break; ++rl; toolong |= (u < 0x80); fallthrough; + case 1: l = seq_sz; break; case 0: l = -1; return 0xFFFD; } - if (l != seq_sz || toolong || u >= limit) + if (rl != seq_sz || toolong || u >= limit) { - l = -l; + l = -rl; return 0xFFFD; } return u; @@ -182,7 +185,8 @@ class _utf_iterator typedef _utf_codec codec; C * cp; - mutable int8_t sl; + mutable int8_t sl, + sz; public: typedef C codeunit_type; @@ -195,18 +199,18 @@ public: reference(const _utf_iterator & i): _i(i) {} public: - operator value_type () const throw () { return codec::get(_i.cp, _i.sl); } - reference & operator = (const value_type usv) throw() { codec::put(_i.cp, usv, _i.sl); return *this; } + operator value_type () const throw () { return codec::get(_i.cp, _i.sl = _i.sz); } + reference & operator = (const value_type usv) throw() { codec::put(_i.cp, usv, _i.sl = _i.sz); return *this; } friend class _utf_iterator; }; - _utf_iterator(const void * us=0) : cp(reinterpret_cast(const_cast(us))), sl(1) { } + _utf_iterator(const void * us=0) : cp(reinterpret_cast(const_cast(us))), sl(1), sz(127) { } _utf_iterator & operator ++ () { cp += abs(sl); return *this; } - bool operator == (const _utf_iterator & rhs) const throw() { return cp >= rhs.cp; } + bool operator == (const _utf_iterator & rhs) const throw() { sz = std::min(ptrdiff_t(rhs.cp - cp),ptrdiff_t(127)); return 0 >= sz; } bool operator != (const _utf_iterator & rhs) const throw() { return !operator==(rhs); } reference operator * () const throw() { return *this; } @@ -245,9 +249,6 @@ struct utf typedef _utf_iterator iterator; typedef _utf_iterator const_iterator; - typedef _utf_sentinal_iterator sentinal_iterator; - typedef _utf_sentinal_iterator const_sentinal_iterator; - inline static bool validate(codeunit_t * s, codeunit_t * e) throw() { return _utf_codec::validate(s,e); @@ -275,9 +276,9 @@ std::basic_string convert(std::basic_string const &src) { using codeunit_t = typename utf::codeunit_t; auto r = std::basic_string(); codeunit_t buf[4]; - int8_t l = 1; + int8_t l = 4; for (auto i = utf_const_iter(src.data()), - e = decltype(i)(src.data() + src.size()); i != e && l > 0; ++i) { + e = decltype(i)(src.data() + src.size()); i != e && l > 0; ++i, l = 4) { utf::codec::put(buf, uchar_t(*i), l); r.append(reinterpret_cast(&buf[0]), l); } diff --git a/common/engine/keyboardprocessor/tests/unit/kmnkbd/context_api.cpp b/common/engine/keyboardprocessor/tests/unit/kmnkbd/context_api.cpp index f7f94acbff..728a21924c 100644 --- a/common/engine/keyboardprocessor/tests/unit/kmnkbd/context_api.cpp +++ b/common/engine/keyboardprocessor/tests/unit/kmnkbd/context_api.cpp @@ -70,9 +70,15 @@ int main(int, char * []) try_status(km_kbp_context_items_to_utf16(ctxt2, ctxt_buffer, &ctxt_size)); if (initial_smp_context != ctxt_buffer) return __LINE__; // Test buffer overrun protection. - ctxt_size=smp_ctxt_size/3; + ctxt_size=4; // This includes space for the null terminator if (km_kbp_context_items_to_utf16(ctxt2, ctxt_buffer, &ctxt_size) - != KM_KBP_STATUS_INSUFFICENT_BUFFER) + != KM_KBP_STATUS_INSUFFICENT_BUFFER + || ctxt_size != 4) + return __LINE__; + ctxt_size=8; // This includes space for the null terminator + if (km_kbp_context_items_to_utf8(ctxt4, ctxt_u8_buffer, &ctxt_size) + != KM_KBP_STATUS_INSUFFICENT_BUFFER + || ctxt_size != 6) return __LINE__; // Create a mock context object and set the items