From e0743a85c5f5f2f83d91c305450e8ba192194cd8 Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Wed, 13 May 2020 09:07:19 +0200 Subject: Forbid negative values in unsigned StrToNum explicitly strtoul(l) surprises us with parsing negative values which should not exist in the places we use to parse them, so we can just downright refuse them rather than trying to work with them by having them promoted to huge positive values. --- apt-pkg/contrib/strutl.cc | 41 +++++++++++++-------------------- test/libapt/strutil_test.cc | 56 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 72 insertions(+), 25 deletions(-) diff --git a/apt-pkg/contrib/strutl.cc b/apt-pkg/contrib/strutl.cc index 7beb5f621..84e79122e 100644 --- a/apt-pkg/contrib/strutl.cc +++ b/apt-pkg/contrib/strutl.cc @@ -23,6 +23,7 @@ #include #include #include +#include #include #include #include @@ -1139,34 +1140,24 @@ bool FTPMDTMStrToTime(const char* const str,time_t &time) /*}}}*/ // StrToNum - Convert a fixed length string to a number /*{{{*/ // --------------------------------------------------------------------- -/* This is used in decoding the crazy fixed length string headers in +/* This is used in decoding the crazy fixed length string headers in tar and ar files. */ bool StrToNum(const char *Str,unsigned long &Res,unsigned Len,unsigned Base) { - char S[30]; - if (Len >= sizeof(S)) + unsigned long long BigRes; + if (not StrToNum(Str, BigRes, Len, Base)) return false; - memcpy(S,Str,Len); - S[Len] = 0; - - // All spaces is a zero - Res = 0; - unsigned I; - for (I = 0; S[I] == ' '; I++); - if (S[I] == 0) - return true; - - char *End; - Res = strtoul(S,&End,Base); - if (End == S) + + if (std::numeric_limits::max() < BigRes) return false; - + + Res = BigRes; return true; } /*}}}*/ // StrToNum - Convert a fixed length string to a number /*{{{*/ // --------------------------------------------------------------------- -/* This is used in decoding the crazy fixed length string headers in +/* This is used in decoding the crazy fixed length string headers in tar and ar files. */ bool StrToNum(const char *Str,unsigned long long &Res,unsigned Len,unsigned Base) { @@ -1175,20 +1166,20 @@ bool StrToNum(const char *Str,unsigned long long &Res,unsigned Len,unsigned Base return false; memcpy(S,Str,Len); S[Len] = 0; - + // All spaces is a zero Res = 0; unsigned I; - for (I = 0; S[I] == ' '; I++); + for (I = 0; S[I] == ' '; ++I); if (S[I] == 0) return true; - + if (S[I] == '-') + return false; + char *End; + errno = 0; Res = strtoull(S,&End,Base); - if (End == S) - return false; - - return true; + return not (End == S || errno != 0); } /*}}}*/ diff --git a/test/libapt/strutil_test.cc b/test/libapt/strutil_test.cc index d477e953c..469de4403 100644 --- a/test/libapt/strutil_test.cc +++ b/test/libapt/strutil_test.cc @@ -1,6 +1,8 @@ #include #include +#include #include +#include #include #include @@ -257,6 +259,60 @@ TEST(StrUtilTest,QuoteString) EXPECT_EQ("Eltville-Erbach", DeQuoteString(QuoteString("Eltville-Erbach", ""))); } +static void EXPECT_STRTONUM(APT::StringView const str, bool const success, unsigned long const expected, unsigned const base) +{ + SCOPED_TRACE(std::string(str.data(), str.length())); + SCOPED_TRACE(base); + unsigned long N1 = 1000; + unsigned long long N2 = 1000; + if (not success) + { + EXPECT_FALSE(StrToNum(str.data(), N1, str.length(), base)); + EXPECT_FALSE(StrToNum(str.data(), N2, str.length(), base)); + return; + } + EXPECT_TRUE(StrToNum(str.data(), N1, str.length(), base)); + EXPECT_EQ(expected, N1); + + EXPECT_TRUE(StrToNum(str.data(), N2, str.length(), base)); + EXPECT_EQ(expected, N2); +} +TEST(StrUtilTest,StrToNum) +{ + EXPECT_STRTONUM("", true, 0, 10); + EXPECT_STRTONUM(" ", true, 0, 10); + EXPECT_STRTONUM("0", true, 0, 10); + EXPECT_STRTONUM("1", true, 1, 10); + EXPECT_STRTONUM(" 1 ", true, 1, 10); + EXPECT_STRTONUM("1", true, 1, 8); + EXPECT_STRTONUM("10", true, 10, 10); + EXPECT_STRTONUM("10", true, 8, 8); + EXPECT_STRTONUM("010", true, 8, 8); + EXPECT_STRTONUM(" 010 ", true, 8, 8); + EXPECT_STRTONUM("-1", false, 0, 10); + EXPECT_STRTONUM(" -1 ", false, 0, 10); + EXPECT_STRTONUM("11", true, 3, 2); + + unsigned long long bigN = 0; + unsigned long smallN = 0; + auto bigLimit = std::to_string(std::numeric_limits::max()); + if (std::numeric_limits::max() < std::numeric_limits::max()) + { + EXPECT_TRUE(StrToNum(bigLimit.c_str(), bigN, bigLimit.length(), 10)); + EXPECT_EQ(std::numeric_limits::max(), bigN); + EXPECT_FALSE(StrToNum(bigLimit.c_str(), smallN, bigLimit.length(), 10)); + } + bigLimit.append("0"); + EXPECT_FALSE(StrToNum(bigLimit.c_str(), bigN, bigLimit.length(), 10)); + EXPECT_FALSE(StrToNum(bigLimit.c_str(), smallN, bigLimit.length(), 10)); + + auto const smallLimit = std::to_string(std::numeric_limits::max()); + EXPECT_TRUE(StrToNum(smallLimit.c_str(), bigN, smallLimit.length(), 10)); + EXPECT_EQ(std::numeric_limits::max(), bigN); + EXPECT_TRUE(StrToNum(smallLimit.c_str(), smallN, smallLimit.length(), 10)); + EXPECT_EQ(std::numeric_limits::max(), smallN); +} + TEST(StrUtilTest,RFC1123StrToTime) { { -- cgit v1.2.3-70-g09d2