From: JINMEI Tatuya Date: Sat, 1 Dec 2012 07:50:33 +0000 (-0800) Subject: [2442] handled various corner cases X-Git-Tag: bind10-1.0.0-beta-release~39^2~3^2~1^2~2^2~17 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=6edf83daa67ac95bb45c2975e0511a830e64ac59;p=thirdparty%2Fkea.git [2442] handled various corner cases --- diff --git a/src/lib/dns/rdata/generic/detail/char_string.cc b/src/lib/dns/rdata/generic/detail/char_string.cc index 841945e41c..e9f900a49b 100644 --- a/src/lib/dns/rdata/generic/detail/char_string.cc +++ b/src/lib/dns/rdata/generic/detail/char_string.cc @@ -12,10 +12,19 @@ // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR // PERFORMANCE OF THIS SOFTWARE. +#include + +#include #include #include +#include + +#include +#include +#include #include + #include namespace isc { @@ -24,18 +33,65 @@ namespace rdata { namespace generic { namespace detail { +namespace { +// Convert a DDD form to the corresponding integer +int +decimalToNumber(const char* s, const char* s_end) { + if (s_end - s < 3) { + isc_throw(InvalidRdataText, "Escaped digits too short"); + } + + // Make a nul-terminated copy of the 'DDD' for lexical_cast + char buf[4]; + std::memcpy(buf, s, 3); + buf[3] = 0; + + try { + const int i = boost::lexical_cast(buf); + if (i > 255) { + isc_throw(InvalidRdataText, "Escaped digits too large: " << buf); + } + return (i); + } catch (const boost::bad_lexical_cast&) { + isc_throw(InvalidRdataText, + "Invalid form for escaped digits: " << buf); + } +} +} + void strToCharString(const MasterToken::StringRegion& str_region, CharString& result) { + // make a space for the 1-byte length field; filled in at the end result.push_back(0); - size_t n = str_region.len; + + bool escape = false; const char* s = str_region.beg; - while (n-- != 0) { - const int c = (*s++) & 0xff; + const char* const s_end = str_region.beg + str_region.len; + + for (size_t n = str_region.len; n != 0; --n, ++s) { + int c = (*s & 0xff); + if (escape && std::isdigit(c) != 0) { + c = decimalToNumber(s, s_end); + assert(n >= 3); + n -= 2; + s += 2; + } else if (!escape && c == '\\') { + escape = true; + continue; + } + escape = false; result.push_back(c); } - result[0] = str_region.len; // FIXME: this is not always correct + if (escape) { // terminated by non-escaped '\' + isc_throw(InvalidRdataText, "character-string ends with '\\'"); + } + if (result.size() > MAX_CHARSTRING_LEN + 1) { // '+ 1' due to the len field + isc_throw(CharStringTooLong, "character-string is too long: " << + result.size() << " bytes"); + } + result[0] = result.size() - 1; } } // end of detail diff --git a/src/lib/dns/tests/rdata_char_string_unittest.cc b/src/lib/dns/tests/rdata_char_string_unittest.cc index fc5346b131..83f0591280 100644 --- a/src/lib/dns/tests/rdata_char_string_unittest.cc +++ b/src/lib/dns/tests/rdata_char_string_unittest.cc @@ -13,6 +13,8 @@ // PERFORMANCE OF THIS SOFTWARE. #include + +#include #include #include @@ -35,19 +37,111 @@ const uint8_t test_charstr[] = { class CharStringTest : public ::testing::Test { protected: CharStringTest() : - test_str("Test String") + // char-string representation for test data using two types of escape + // ('r' = 114) + test_str("Test\\ St\\114ing") { str_region.beg = &test_str[0]; str_region.len = test_str.size(); } - CharString chstr; // placeholder + CharString chstr; // place holder const std::string test_str; MasterToken::StringRegion str_region; }; -TEST_F(CharStringTest, test) { +MasterToken::StringRegion +createStringRegion(const std::string& str) { + MasterToken::StringRegion region; + region.beg = &str[0]; // note this works even if str is empty + region.len = str.size(); + return (region); +} + +TEST_F(CharStringTest, normalConversion) { + uint8_t tmp[3]; // placeholder for expected sequence + strToCharString(str_region, chstr); matchWireData(test_charstr, sizeof(test_charstr), &chstr[0], chstr.size()); + + // Empty string + chstr.clear(); + strToCharString(createStringRegion(""), chstr); + tmp[0] = 0; + matchWireData(tmp, 1, &chstr[0], chstr.size()); + + // Possible largest char string + chstr.clear(); + std::string long_str(255, 'x'); + strToCharString(createStringRegion(long_str), chstr); + std::vector expected; + expected.push_back(255); // len of char string + expected.insert(expected.end(), long_str.begin(), long_str.end()); + matchWireData(&expected[0], expected.size(), &chstr[0], chstr.size()); + + // Same data as the previous case, but the original string is longer than + // the max; this shouldn't be rejected + chstr.clear(); + long_str.at(254) = '\\'; // replace the last 'x' with '\' + long_str.append("120"); // 'x' = 120 + strToCharString(createStringRegion(long_str), chstr); + matchWireData(&expected[0], expected.size(), &chstr[0], chstr.size()); + + // Escaped '\' + chstr.clear(); + tmp[0] = 1; + tmp[1] = '\\'; + strToCharString(createStringRegion("\\\\"), chstr); + matchWireData(tmp, 2, &chstr[0], chstr.size()); + + // Boundary values for \DDD + chstr.clear(); + tmp[0] = 1; + tmp[1] = 0; + strToCharString(createStringRegion("\\000"), chstr); + matchWireData(tmp, 2, &chstr[0], chstr.size()); + + chstr.clear(); + strToCharString(createStringRegion("\\255"), chstr); + tmp[0] = 1; + tmp[1] = 255; + matchWireData(tmp, 2, &chstr[0], chstr.size()); + + // Another digit follows DDD; it shouldn't cause confusion + chstr.clear(); + strToCharString(createStringRegion("\\2550"), chstr); + tmp[0] = 2; // string len is now 2 + tmp[2] = '0'; + matchWireData(tmp, 3, &chstr[0], chstr.size()); +} + +TEST_F(CharStringTest, badConversion) { + // string cannot exceed 255 bytes + EXPECT_THROW(strToCharString(createStringRegion(std::string(256, 'a')), + chstr), + CharStringTooLong); + + // input string ending with (non escaped) '\' + chstr.clear(); + EXPECT_THROW(strToCharString(createStringRegion("foo\\"), chstr), + InvalidRdataText); +} + +TEST_F(CharStringTest, badDDD) { + // Check various type of bad form of \DDD + + // Not a number + EXPECT_THROW(strToCharString(createStringRegion("\\1a2"), chstr), + InvalidRdataText); + EXPECT_THROW(strToCharString(createStringRegion("\\12a"), chstr), + InvalidRdataText); + + // Not in the range of uint8_t + EXPECT_THROW(strToCharString(createStringRegion("\\256"), chstr), + InvalidRdataText); + + // Short buffer + EXPECT_THROW(strToCharString(createStringRegion("\\42"), chstr), + InvalidRdataText); } } // unnamed namespace