From 89f3991351185b365ccd10525e74d35d7bb2da46 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lars=20M=C3=BCller?= <34514239+appgurueu@users.noreply.github.com> Date: Sun, 30 May 2021 20:23:12 +0200 Subject: [PATCH] Fix base64 validation and add unittests (#10515) Implement proper padding character checks --- doc/client_lua_api.txt | 4 +- doc/lua_api.txt | 4 +- src/unittest/test_utilities.cpp | 93 +++++++++++++++++++++++++++++++++ src/util/base64.cpp | 29 ++++++++-- 4 files changed, 124 insertions(+), 6 deletions(-) diff --git a/doc/client_lua_api.txt b/doc/client_lua_api.txt index 1e8015f7b..d239594f7 100644 --- a/doc/client_lua_api.txt +++ b/doc/client_lua_api.txt @@ -910,7 +910,9 @@ Call these functions only at load time! * Example: `minetest.rgba(10, 20, 30, 40)`, returns `"#0A141E28"` * `minetest.encode_base64(string)`: returns string encoded in base64 * Encodes a string in base64. -* `minetest.decode_base64(string)`: returns string +* `minetest.decode_base64(string)`: returns string or nil on failure + * Padding characters are only supported starting at version 5.4.0, where + 5.5.0 and newer perform proper checks. * Decodes a string encoded in base64. * `minetest.gettext(string)` : returns string * look up the translation of a string in the gettext message catalog diff --git a/doc/lua_api.txt b/doc/lua_api.txt index 6c7ae0fb5..956919c89 100644 --- a/doc/lua_api.txt +++ b/doc/lua_api.txt @@ -5773,7 +5773,9 @@ Misc. * Example: `minetest.rgba(10, 20, 30, 40)`, returns `"#0A141E28"` * `minetest.encode_base64(string)`: returns string encoded in base64 * Encodes a string in base64. -* `minetest.decode_base64(string)`: returns string or nil for invalid base64 +* `minetest.decode_base64(string)`: returns string or nil on failure + * Padding characters are only supported starting at version 5.4.0, where + 5.5.0 and newer perform proper checks. * Decodes a string encoded in base64. * `minetest.is_protected(pos, name)`: returns boolean * Returning `true` restricts the player `name` from modifying (i.e. digging, diff --git a/src/unittest/test_utilities.cpp b/src/unittest/test_utilities.cpp index 93ba3f844..039110d54 100644 --- a/src/unittest/test_utilities.cpp +++ b/src/unittest/test_utilities.cpp @@ -23,6 +23,7 @@ with this program; if not, write to the Free Software Foundation, Inc., #include "util/enriched_string.h" #include "util/numeric.h" #include "util/string.h" +#include "util/base64.h" class TestUtilities : public TestBase { public: @@ -56,6 +57,7 @@ public: void testMyround(); void testStringJoin(); void testEulerConversion(); + void testBase64(); }; static TestUtilities g_test_instance; @@ -87,6 +89,7 @@ void TestUtilities::runTests(IGameDef *gamedef) TEST(testMyround); TEST(testStringJoin); TEST(testEulerConversion); + TEST(testBase64); } //////////////////////////////////////////////////////////////////////////////// @@ -537,3 +540,93 @@ void TestUtilities::testEulerConversion() setPitchYawRoll(m2, v2); UASSERT(within(m1, m2, tolL)); } + +void TestUtilities::testBase64() +{ + // Test character set + UASSERT(base64_is_valid("ABCDEFGHIJKLMNOPQRSTUVWXYZ" + "abcdefghijklmnopqrstuvwxyz" + "0123456789+/") == true); + UASSERT(base64_is_valid("/+9876543210" + "zyxwvutsrqponmlkjihgfedcba" + "ZYXWVUTSRQPONMLKJIHGFEDCBA") == true); + UASSERT(base64_is_valid("ABCDEFGHIJKLMNOPQRSTUVWXYZ" + "abcdefghijklmnopqrstuvwxyz" + "0123456789+.") == false); + UASSERT(base64_is_valid("ABCDEFGHIJKLMNOPQRSTUVWXYZ" + "abcdefghijklmnopqrstuvwxyz" + "0123456789 /") == false); + + // Test empty string + UASSERT(base64_is_valid("") == true); + + // Test different lengths, with and without padding, + // with correct and incorrect padding + UASSERT(base64_is_valid("A") == false); + UASSERT(base64_is_valid("AA") == true); + UASSERT(base64_is_valid("AAA") == true); + UASSERT(base64_is_valid("AAAA") == true); + UASSERT(base64_is_valid("AAAAA") == false); + UASSERT(base64_is_valid("AAAAAA") == true); + UASSERT(base64_is_valid("AAAAAAA") == true); + UASSERT(base64_is_valid("AAAAAAAA") == true); + UASSERT(base64_is_valid("A===") == false); + UASSERT(base64_is_valid("AA==") == true); + UASSERT(base64_is_valid("AAA=") == true); + UASSERT(base64_is_valid("AAAA") == true); + UASSERT(base64_is_valid("AAAA====") == false); + UASSERT(base64_is_valid("AAAAA===") == false); + UASSERT(base64_is_valid("AAAAAA==") == true); + UASSERT(base64_is_valid("AAAAAAA=") == true); + UASSERT(base64_is_valid("AAAAAAA==") == false); + UASSERT(base64_is_valid("AAAAAAA===") == false); + UASSERT(base64_is_valid("AAAAAAA====") == false); + UASSERT(base64_is_valid("AAAAAAAA") == true); + UASSERT(base64_is_valid("AAAAAAAA=") == false); + UASSERT(base64_is_valid("AAAAAAAA==") == false); + UASSERT(base64_is_valid("AAAAAAAA===") == false); + UASSERT(base64_is_valid("AAAAAAAA====") == false); + + // Test if canonical encoding + // Last character limitations, length % 4 == 3 + UASSERT(base64_is_valid("AAB") == false); + UASSERT(base64_is_valid("AAE") == true); + UASSERT(base64_is_valid("AAQ") == true); + UASSERT(base64_is_valid("AAB=") == false); + UASSERT(base64_is_valid("AAE=") == true); + UASSERT(base64_is_valid("AAQ=") == true); + UASSERT(base64_is_valid("AAAAAAB=") == false); + UASSERT(base64_is_valid("AAAAAAE=") == true); + UASSERT(base64_is_valid("AAAAAAQ=") == true); + // Last character limitations, length % 4 == 2 + UASSERT(base64_is_valid("AB") == false); + UASSERT(base64_is_valid("AE") == false); + UASSERT(base64_is_valid("AQ") == true); + UASSERT(base64_is_valid("AB==") == false); + UASSERT(base64_is_valid("AE==") == false); + UASSERT(base64_is_valid("AQ==") == true); + UASSERT(base64_is_valid("AAAAAB==") == false); + UASSERT(base64_is_valid("AAAAAE==") == false); + UASSERT(base64_is_valid("AAAAAQ==") == true); + + // Extraneous character present + UASSERT(base64_is_valid(".") == false); + UASSERT(base64_is_valid("A.") == false); + UASSERT(base64_is_valid("AA.") == false); + UASSERT(base64_is_valid("AAA.") == false); + UASSERT(base64_is_valid("AAAA.") == false); + UASSERT(base64_is_valid("AAAAA.") == false); + UASSERT(base64_is_valid("A.A") == false); + UASSERT(base64_is_valid("AA.A") == false); + UASSERT(base64_is_valid("AAA.A") == false); + UASSERT(base64_is_valid("AAAA.A") == false); + UASSERT(base64_is_valid("AAAAA.A") == false); + UASSERT(base64_is_valid("\xE1""AAA") == false); + + // Padding in wrong position + UASSERT(base64_is_valid("A=A") == false); + UASSERT(base64_is_valid("AA=A") == false); + UASSERT(base64_is_valid("AAA=A") == false); + UASSERT(base64_is_valid("AAAA=A") == false); + UASSERT(base64_is_valid("AAAAA=A") == false); +} \ No newline at end of file diff --git a/src/util/base64.cpp b/src/util/base64.cpp index 6e1584410..0c2455222 100644 --- a/src/util/base64.cpp +++ b/src/util/base64.cpp @@ -33,18 +33,39 @@ static const std::string base64_chars = "abcdefghijklmnopqrstuvwxyz" "0123456789+/"; +static const std::string base64_chars_padding_1 = "AEIMQUYcgkosw048"; +static const std::string base64_chars_padding_2 = "AQgw"; static inline bool is_base64(unsigned char c) { - return isalnum(c) || c == '+' || c == '/' || c == '='; + return (c >= '0' && c <= '9') + || (c >= 'A' && c <= 'Z') + || (c >= 'a' && c <= 'z') + || c == '+' || c == '/'; } bool base64_is_valid(std::string const& s) { - for (char i : s) - if (!is_base64(i)) + size_t i = 0; + for (; i < s.size(); ++i) + if (!is_base64(s[i])) + break; + unsigned char padding = 3 - ((i + 3) % 4); + if ((padding == 1 && base64_chars_padding_1.find(s[i - 1]) == std::string::npos) + || (padding == 2 && base64_chars_padding_2.find(s[i - 1]) == std::string::npos) + || padding == 3) + return false; + int actual_padding = s.size() - i; + // omission of padding characters is allowed + if (actual_padding == 0) + return true; + + // remaining characters (max. 2) may only be padding + for (; i < s.size(); ++i) + if (s[i] != '=') return false; - return true; + // number of padding characters needs to match + return padding == actual_padding; } std::string base64_encode(unsigned char const* bytes_to_encode, unsigned int in_len) {