fix: tr_clientForId() (#2887)

* fix: memory corruption when parsing negative numbers

* fix: stack-buffer-overflow on escape chars in tr_clientForId

* test: add tr_clientForId() fuzz tests

libtransmission/clients.cc

@@ -72,24 +72,112 @@ constexpr std::pair<char*, size_t> buf_append(char* buf, size_t buflen, T t, Arg
     return buf_append(buf, buflen, args...);
 }
 
-// ['0'..'9']: ch - '0'
+constexpr std::string_view charint(char ch)
-// ['A'..'Z']: 10 + ch - '9'
+{
-// ['a'..'z']: 36 + ch - '9'
+    switch (ch)
-auto constexpr charints = std::array<std::string_view, 256>{
+    {
-    { "x",  "x",  "x",  "x",  "x",  "x",  "x",  "x",  "x",  "x",  "x",  "x",  "x",  "x",  "x",  "x",  "x",  "x",  "x",  "x",
+    case '0':
-      "x",  "x",  "x",  "x",  "x",  "x",  "x",  "x",  "x",  "x",  "x",  "x",  "x",  "x",  "x",  "x",  "x",  "x",  "x",  "x",
+        return "0"sv;
-      "x",  "x",  "x",  "x",  "x",  "x",  "x",  "x",  "0",  "1",  "2",  "3",  "4",  "5",  "6",  "7",  "8",  "9",  "x",  "x",
+    case '1':
-      "x",  "x",  "x",  "x",  "x",  "10", "11", "12", "13", "14", "15", "16", "17", "18", "19", "20", "21", "22", "23", "24",
+        return "1"sv;
-      "25", "26", "27", "28", "29", "30", "31", "32", "33", "34", "35", "x",  "x",  "x",  "x",  "x",  "x",  "36", "37", "38",
+    case '2':
-      "39", "40", "41", "42", "43", "44", "45", "46", "47", "48", "49", "50", "51", "52", "53", "54", "55", "56", "57", "58",
+        return "2"sv;
-      "59", "60", "61", "x",  "x",  "x",  "x",  "x",  "x",  "x",  "x",  "x",  "x",  "x",  "x",  "x",  "x",  "x",  "x",  "x",
+    case '3':
-      "x",  "x",  "x",  "x",  "x",  "x",  "x",  "x",  "x",  "x",  "x",  "x",  "x",  "x",  "x",  "x",  "x",  "x",  "x",  "x",
+        return "3"sv;
-      "x",  "x",  "x",  "x",  "x",  "x",  "x",  "x",  "x",  "x",  "x",  "x",  "x",  "x",  "x",  "x",  "x",  "x",  "x",  "x",
+    case '4':
-      "x",  "x",  "x",  "x",  "x",  "x",  "x",  "x",  "x",  "x",  "x",  "x",  "x",  "x",  "x",  "x",  "x",  "x",  "x",  "x",
+        return "4"sv;
-      "x",  "x",  "x",  "x",  "x",  "x",  "x",  "x",  "x",  "x",  "x",  "x",  "x",  "x",  "x",  "x",  "x",  "x",  "x",  "x",
+    case '5':
-      "x",  "x",  "x",  "x",  "x",  "x",  "x",  "x",  "x",  "x",  "x",  "x",  "x",  "x",  "x",  "x",  "x",  "x",  "x",  "x",
+        return "5"sv;
-      "x",  "x",  "x",  "x",  "x",  "x",  "x",  "x",  "x",  "x",  "x",  "x",  "x",  "x",  "x",  "x" }
+    case '6':
-};
+        return "6"sv;
+    case '7':
+        return "7"sv;
+    case '8':
+        return "8"sv;
+    case '9':
+        return "9"sv;
+    case 'a':
+    case 'A':
+        return "10"sv;
+    case 'b':
+    case 'B':
+        return "11"sv;
+    case 'c':
+    case 'C':
+        return "12"sv;
+    case 'd':
+    case 'D':
+        return "13"sv;
+    case 'e':
+    case 'E':
+        return "14"sv;
+    case 'f':
+    case 'F':
+        return "15"sv;
+    case 'g':
+    case 'G':
+        return "16"sv;
+    case 'h':
+    case 'H':
+        return "17"sv;
+    case 'i':
+    case 'I':
+        return "18"sv;
+    case 'j':
+    case 'J':
+        return "19"sv;
+    case 'k':
+    case 'K':
+        return "20"sv;
+    case 'l':
+    case 'L':
+        return "21"sv;
+    case 'm':
+    case 'M':
+        return "22"sv;
+    case 'n':
+    case 'N':
+        return "23"sv;
+    case 'o':
+    case 'O':
+        return "24"sv;
+    case 'p':
+    case 'P':
+        return "25"sv;
+    case 'q':
+    case 'Q':
+        return "26"sv;
+    case 'r':
+    case 'R':
+        return "27"sv;
+    case 's':
+    case 'S':
+        return "28"sv;
+    case 't':
+    case 'T':
+        return "29"sv;
+    case 'u':
+    case 'U':
+        return "30"sv;
+    case 'v':
+    case 'V':
+        return "31"sv;
+    case 'w':
+    case 'W':
+        return "32"sv;
+    case 'x':
+    case 'X':
+        return "33"sv;
+    case 'y':
+    case 'Y':
+        return "34"sv;
+    case 'z':
+    case 'Z':
+        return "35"sv;
+    default:
+        return "x"sv;
+    }
+}
 
 int strint(void const* pch, int span, int base = 0)
 {
@@ -249,15 +337,15 @@ using format_func = void (*)(char* buf, size_t buflen, std::string_view name, tr
 
 constexpr void three_digit_formatter(char* buf, size_t buflen, std::string_view name, tr_peer_id_t id)
 {
-    buf_append(buf, buflen, name, ' ', charints[id[3]], '.', charints[id[4]], '.', charints[id[5]]);
+    buf_append(buf, buflen, name, ' ', charint(id[3]), '.', charint(id[4]), '.', charint(id[5]));
 }
 
 constexpr void four_digit_formatter(char* buf, size_t buflen, std::string_view name, tr_peer_id_t id)
 {
-    buf_append(buf, buflen, name, ' ', charints[id[3]], '.', charints[id[4]], '.', charints[id[5]], '.', charints[id[6]]);
+    buf_append(buf, buflen, name, ' ', charint(id[3]), '.', charint(id[4]), '.', charint(id[5]), '.', charint(id[6]));
 }
 
-constexpr void no_version_formatter(char* buf, size_t buflen, std::string_view name, tr_peer_id_t /*id*/)
+void no_version_formatter(char* buf, size_t buflen, std::string_view name, tr_peer_id_t /*id*/)
 {
     buf_append(buf, buflen, name);
 }
@@ -344,23 +432,23 @@ constexpr void burst_formatter(char* buf, size_t buflen, std::string_view name,
 
 constexpr void ctorrent_formatter(char* buf, size_t buflen, std::string_view name, tr_peer_id_t id)
 {
-    buf_append(buf, buflen, name, ' ', charints[id[3]], '.', charints[id[4]], '.', id[5], id[6]);
+    buf_append(buf, buflen, name, ' ', charint(id[3]), '.', charint(id[4]), '.', id[5], id[6]);
 }
 
 constexpr void folx_formatter(char* buf, size_t buflen, std::string_view name, tr_peer_id_t id)
 {
-    buf_append(buf, buflen, name, ' ', charints[id[3]], '.', 'x');
+    buf_append(buf, buflen, name, ' ', charint(id[3]), '.', 'x');
 }
 
 constexpr void ktorrent_formatter(char* buf, size_t buflen, std::string_view name, tr_peer_id_t id)
 {
     if (id[5] == 'D')
     {
-        buf_append(buf, buflen, name, ' ', charints[id[3]], '.', charints[id[4]], " Dev "sv, charints[id[6]]);
+        buf_append(buf, buflen, name, ' ', charint(id[3]), '.', charint(id[4]), " Dev "sv, charint(id[6]));
     }
     else if (id[5] == 'R')
     {
-        buf_append(buf, buflen, name, ' ', charints[id[3]], '.', charints[id[4]], " RC "sv, charints[id[6]]);
+        buf_append(buf, buflen, name, ' ', charint(id[3]), '.', charint(id[4]), " RC "sv, charint(id[6]));
     }
     else
     {
@@ -389,7 +477,7 @@ constexpr void mainline_formatter(char* buf, size_t buflen, std::string_view nam
 
 constexpr void mediaget_formatter(char* buf, size_t buflen, std::string_view name, tr_peer_id_t id)
 {
-    buf_append(buf, buflen, name, ' ', charints[id[3]], '.', charints[id[4]]);
+    buf_append(buf, buflen, name, ' ', charint(id[3]), '.', charint(id[4]));
 }
 
 constexpr void mldonkey_formatter(char* buf, size_t buflen, std::string_view name, tr_peer_id_t id)
@@ -411,7 +499,7 @@ constexpr void opera_formatter(char* buf, size_t buflen, std::string_view name,
 
 constexpr void picotorrent_formatter(char* buf, size_t buflen, std::string_view name, tr_peer_id_t id)
 {
-    buf_append(buf, buflen, name, ' ', charints[id[3]], '.', id[4], id[5], '.', charints[id[6]]);
+    buf_append(buf, buflen, name, ' ', charint(id[3]), '.', id[4], id[5], '.', charint(id[6]));
 }
 
 constexpr void plus_formatter(char* buf, size_t buflen, std::string_view name, tr_peer_id_t id)
@@ -421,7 +509,7 @@ constexpr void plus_formatter(char* buf, size_t buflen, std::string_view name, t
 
 constexpr void qvod_formatter(char* buf, size_t buflen, std::string_view name, tr_peer_id_t id)
 {
-    buf_append(buf, buflen, name, ' ', charints[id[4]], '.', charints[id[5]], '.', charints[id[6]], '.', charints[id[7]]);
+    buf_append(buf, buflen, name, ' ', charint(id[4]), '.', charint(id[5]), '.', charint(id[6]), '.', charint(id[7]));
 }
 
 void transmission_formatter(char* buf, size_t buflen, std::string_view name, tr_peer_id_t id)
@@ -500,7 +588,7 @@ constexpr void xfplay_formatter(char* buf, size_t buflen, std::string_view name,
 
 void xtorrent_formatter(char* buf, size_t buflen, std::string_view name, tr_peer_id_t id)
 {
-    std::tie(buf, buflen) = buf_append(buf, buflen, name, ' ', charints[id[3]], '.', charints[id[4]], " ("sv);
+    std::tie(buf, buflen) = buf_append(buf, buflen, name, ' ', charint(id[3]), '.', charint(id[4]), " ("sv);
     *fmt::format_to_n(buf, buflen - 1, FMT_STRING("{:d}"), strint(&id[5], 2)).out = '\0';
 }
 
@@ -699,7 +787,7 @@ char* tr_clientForId(char* buf, size_t buflen, tr_peer_id_t peer_id)
             }
             else
             {
-                walk = fmt::format_to_n(walk, end - walk - 1, FMT_STRING("%{:02X}"), unsigned(c)).out;
+                walk = fmt::format_to_n(walk, end - walk - 1, FMT_STRING("%{:02X}"), static_cast<unsigned char>(c)).out;
             }
         }
 

tests/libtransmission/clients-test.cc

@@ -74,3 +74,31 @@ TEST(Client, clientForId)
         EXPECT_EQ(test.expected_client, std::string_view{ buf.data() });
     }
 }
+
+TEST(Client, fuzzRegressions)
+{
+    auto constexpr Tests = std::array<std::string_view, 5>{
+        "LVJTp3u+Aptl01HjzTHXVC5b9g4="sv, "LWJrHb2OpoNsJdODHA7iyXjnHxc="sv, "LU1PjpTjmvUth+f15YTOOggXl3k="sv,
+        "LUxU1gO7xhfBD4bmyZkB+neZIx0="sv, "LVJTp3u+Aptl01HjzTHXVC5b9g4="sv,
+    };
+
+    for (auto const& test : Tests)
+    {
+        auto const input = tr_base64_decode(test);
+        auto peer_id = tr_peer_id_t{};
+        std::copy(std::begin(input), std::end(input), std::begin(peer_id));
+        auto buf = std::array<char, 128>{};
+        tr_clientForId(buf.data(), buf.size(), peer_id);
+    }
+}
+
+TEST(Client, fuzz)
+{
+    for (size_t i = 0; i < 10000; ++i)
+    {
+        auto peer_id = tr_peer_id_t{};
+        tr_rand_buffer(std::data(peer_id), std::size(peer_id));
+        auto buf = std::array<char, 128>{};
+        tr_clientForId(buf.data(), buf.size(), peer_id);
+    }
+}