From 495410c450b4c46399c6cf5367f5ff513abbbd71 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Klemens=20B=C3=B6swirth?= Date: Fri, 2 Oct 2020 20:34:50 +0200 Subject: [PATCH] fix bug in keyname validation --- src/libs/elektra/keyname.c | 38 +++++++++---- src/tools/kdb/test.cpp | 1 + tests/ctest/test_keyname.c | 111 ++++++++++++++++++++++++++++++++++++- 3 files changed, 136 insertions(+), 14 deletions(-) diff --git a/src/libs/elektra/keyname.c b/src/libs/elektra/keyname.c index 2456a1de716..c79b3a85513 100644 --- a/src/libs/elektra/keyname.c +++ b/src/libs/elektra/keyname.c @@ -515,15 +515,15 @@ int elektraKeyNameValidate (const char * name, const char * prefix, size_t * siz } } - // validate path in reverse because of .. + // need to validate path in reverse because of /../ parts name = pathStart + pathLen - 1; // check backslashes at end of name + // this ONLY checks for dangling escapes, size adjustments are done in the main loop size_t backslashes = 0; - while (name >= pathStart && *name == '\\') + while (name - backslashes >= pathStart && *(name - backslashes) == '\\') { ++backslashes; - --name; } if (backslashes % 2 != 0) @@ -531,7 +531,6 @@ int elektraKeyNameValidate (const char * name, const char * prefix, size_t * siz // dangling escape return 0; } - usize -= backslashes / 2; enum { @@ -548,17 +547,38 @@ int elektraKeyNameValidate (const char * name, const char * prefix, size_t * siz size_t skip = 0; while (name >= pathStart) { - char cur = *name; + size_t trailingSlashes = 0; + if (state == PART_END) + { + // count trailing slashes (the last one might be escaped) + while (name - trailingSlashes >= pathStart && *(name - trailingSlashes) == '/') + { + ++trailingSlashes; + } + } // count upcomming backslashes backslashes = 0; - while (name - 1 - backslashes >= pathStart && *(name - 1 - backslashes) == '\\') + size_t backslashOffset = trailingSlashes == 0 ? 1 : trailingSlashes; + while (name - backslashOffset - backslashes >= pathStart && *(name - backslashOffset - backslashes) == '\\') { ++backslashes; } int escaped = backslashes % 2 != 0; + // the last trailing slash is actually escaped + if (trailingSlashes > 0 && escaped) + { + --trailingSlashes; + } + + name -= trailingSlashes; + size -= trailingSlashes; + usize -= trailingSlashes; + + char cur = *name; + switch (state) { case PART_END: @@ -568,11 +588,6 @@ int elektraKeyNameValidate (const char * name, const char * prefix, size_t * siz partLen = 1; state = DIGITS; } - else if (cur == '/' && !escaped) - { - --size; - --usize; - } else { partLen = 1; @@ -764,7 +779,6 @@ int elektraKeyNameValidate (const char * name, const char * prefix, size_t * siz ++skip; } - digits = 0; underscores = 0; diff --git a/src/tools/kdb/test.cpp b/src/tools/kdb/test.cpp index 1c20bc46231..20bfae157e4 100644 --- a/src/tools/kdb/test.cpp +++ b/src/tools/kdb/test.cpp @@ -269,6 +269,7 @@ void TestCommand::doBinaryTest () void TestCommand::doNamingTest () { vector teststrings; + teststrings.push_back ("\\"); teststrings.push_back ("keyname"); teststrings.push_back ("deep/below/keyname"); teststrings.push_back ("keyname with spaces"); diff --git a/tests/ctest/test_keyname.c b/tests/ctest/test_keyname.c index a9a46b467b3..ef4109c3a0e 100644 --- a/tests/ctest/test_keyname.c +++ b/tests/ctest/test_keyname.c @@ -66,6 +66,60 @@ static void test_validate (void) TEST_VALIDATE_OK ("/\\\\\\\\/", NULL, 0, 6, 5); TEST_VALIDATE_OK ("/\\\\\\\\\\/", NULL, 0, 8, 6); + TEST_VALIDATE_OK ("/\\/", NULL, 0, 4, 4); + TEST_VALIDATE_OK ("/\\\\", NULL, 0, 4, 4); + TEST_VALIDATE_OK ("/\\\\\\/", NULL, 0, 6, 5); + TEST_VALIDATE_OK ("/\\\\\\\\", NULL, 0, 6, 5); + TEST_VALIDATE_OK ("/\\\\\\\\\\/", NULL, 0, 8, 6); + + TEST_VALIDATE_OK ("/\\//", NULL, 0, 4, 4); + TEST_VALIDATE_OK ("/\\\\/", NULL, 0, 4, 4); + TEST_VALIDATE_OK ("/\\\\\\//", NULL, 0, 6, 5); + TEST_VALIDATE_OK ("/\\\\\\\\/", NULL, 0, 6, 5); + TEST_VALIDATE_OK ("/\\\\\\\\\\//", NULL, 0, 8, 6); + + TEST_VALIDATE_OK ("user:/\\/", NULL, 0, 9, 4); + TEST_VALIDATE_OK ("user:/\\\\", NULL, 0, 9, 4); + TEST_VALIDATE_OK ("user:/\\\\\\/", NULL, 0, 11, 5); + TEST_VALIDATE_OK ("user:/\\\\\\\\", NULL, 0, 11, 5); + TEST_VALIDATE_OK ("user:/\\\\\\\\\\/", NULL, 0, 13, 6); + + TEST_VALIDATE_OK ("user:/\\//", NULL, 0, 9, 4); + TEST_VALIDATE_OK ("user:/\\\\/", NULL, 0, 9, 4); + TEST_VALIDATE_OK ("user:/\\\\\\//", NULL, 0, 11, 5); + TEST_VALIDATE_OK ("user:/\\\\\\\\/", NULL, 0, 11, 5); + TEST_VALIDATE_OK ("user:/\\\\\\\\\\//", NULL, 0, 13, 6); + + TEST_VALIDATE_OK ("user:/tests/plugin/\\/", NULL, 0, 22, 17); + TEST_VALIDATE_OK ("user:/tests/plugin/\\\\", NULL, 0, 22, 17); + TEST_VALIDATE_OK ("user:/tests/plugin/\\\\\\/", NULL, 0, 24, 18); + TEST_VALIDATE_OK ("user:/tests/plugin/\\\\\\\\", NULL, 0, 24, 18); + TEST_VALIDATE_OK ("user:/tests/plugin/\\\\\\\\\\/", NULL, 0, 26, 19); + + TEST_VALIDATE_OK ("user:/tests/plugin/\\//", NULL, 0, 22, 17); + TEST_VALIDATE_OK ("user:/tests/plugin/\\\\/", NULL, 0, 22, 17); + TEST_VALIDATE_OK ("user:/tests/plugin/\\\\\\//", NULL, 0, 24, 18); + TEST_VALIDATE_OK ("user:/tests/plugin/\\\\\\\\/", NULL, 0, 24, 18); + TEST_VALIDATE_OK ("user:/tests/plugin/\\\\\\\\\\//", NULL, 0, 26, 19); + + TEST_VALIDATE_OK ("/\\//abc", NULL, 0, 8, 8); + TEST_VALIDATE_OK ("/\\\\/abc", NULL, 0, 8, 8); + TEST_VALIDATE_OK ("/\\\\\\//abc", NULL, 0, 10, 9); + TEST_VALIDATE_OK ("/\\\\\\\\/abc", NULL, 0, 10, 9); + TEST_VALIDATE_OK ("/\\\\\\\\\\//abc", NULL, 0, 12, 10); + + TEST_VALIDATE_OK ("user:/\\//abc", NULL, 0, 13, 8); + TEST_VALIDATE_OK ("user:/\\\\/abc", NULL, 0, 13, 8); + TEST_VALIDATE_OK ("user:/\\\\\\//abc", NULL, 0, 15, 9); + TEST_VALIDATE_OK ("user:/\\\\\\\\/abc", NULL, 0, 15, 9); + TEST_VALIDATE_OK ("user:/\\\\\\\\\\//abc", NULL, 0, 17, 10); + + TEST_VALIDATE_OK ("user:/tests/plugin/\\//abc", NULL, 0, 26, 21); + TEST_VALIDATE_OK ("user:/tests/plugin/\\\\/abc", NULL, 0, 26, 21); + TEST_VALIDATE_OK ("user:/tests/plugin/\\\\\\//abc", NULL, 0, 28, 22); + TEST_VALIDATE_OK ("user:/tests/plugin/\\\\\\\\/abc", NULL, 0, 28, 22); + TEST_VALIDATE_OK ("user:/tests/plugin/\\\\\\\\\\//abc", NULL, 0, 30, 23); + TEST_VALIDATE_OK ("/\\///", NULL, 0, 4, 4); TEST_VALIDATE_OK ("/\\/\\/", NULL, 0, 6, 5); @@ -81,6 +135,9 @@ static void test_validate (void) TEST_VALIDATE_OK ("/////////", NULL, 0, 2, 3); TEST_VALIDATE_OK ("user://///////", NULL, 0, 7, 3); + TEST_VALIDATE_OK ("/a////////", NULL, 0, 3, 4); + TEST_VALIDATE_OK ("user:/a////////", NULL, 0, 8, 4); + TEST_VALIDATE_OK ("/abc/%/def", NULL, 0, 11, 11); TEST_VALIDATE_OK ("user:/abc/%/def", NULL, 0, 16, 11); @@ -354,6 +411,9 @@ static void test_validate (void) TEST_VALIDATE_ERROR ("/\\\\\\\\\\%", NULL, 0); } +#undef TEST_VALIDATE_OK +#undef TEST_VALIDATE_ERROR + #define TEST_CANONICALIZE_OK(name, prefix, cname) \ do \ { \ @@ -524,17 +584,49 @@ static void test_canonicalize (void) TEST_CANONICALIZE_OK ("/\\\\\\\\%", "", "/\\\\\\\\%"); TEST_CANONICALIZE_OK ("/\\/", "", "/\\/"); - TEST_CANONICALIZE_OK ("/\\\\/", "", "/\\\\"); + TEST_CANONICALIZE_OK ("/\\\\", "", "/\\\\"); TEST_CANONICALIZE_OK ("/\\\\\\/", "", "/\\\\\\/"); - TEST_CANONICALIZE_OK ("/\\\\\\\\/", "", "/\\\\\\\\"); + TEST_CANONICALIZE_OK ("/\\\\\\\\", "", "/\\\\\\\\"); TEST_CANONICALIZE_OK ("/\\\\\\\\\\/", "", "/\\\\\\\\\\/"); + TEST_CANONICALIZE_OK ("/\\//", "", "/\\/"); + TEST_CANONICALIZE_OK ("/\\\\/", "", "/\\\\"); + TEST_CANONICALIZE_OK ("/\\\\\\//", "", "/\\\\\\/"); + TEST_CANONICALIZE_OK ("/\\\\\\\\/", "", "/\\\\\\\\"); + TEST_CANONICALIZE_OK ("/\\\\\\\\\\//", "", "/\\\\\\\\\\/"); + + TEST_CANONICALIZE_OK ("user:/\\/", "", "user:/\\/"); + TEST_CANONICALIZE_OK ("user:/\\\\", "", "user:/\\\\"); + TEST_CANONICALIZE_OK ("user:/\\\\\\/", "", "user:/\\\\\\/"); + TEST_CANONICALIZE_OK ("user:/\\\\\\\\", "", "user:/\\\\\\\\"); + TEST_CANONICALIZE_OK ("user:/\\\\\\\\\\/", "", "user:/\\\\\\\\\\/"); + + TEST_CANONICALIZE_OK ("user:/\\//", "", "user:/\\/"); + TEST_CANONICALIZE_OK ("user:/\\\\/", "", "user:/\\\\"); + TEST_CANONICALIZE_OK ("user:/\\\\\\//", "", "user:/\\\\\\/"); + TEST_CANONICALIZE_OK ("user:/\\\\\\\\/", "", "user:/\\\\\\\\"); + TEST_CANONICALIZE_OK ("user:/\\\\\\\\\\//", "", "user:/\\\\\\\\\\/"); + + TEST_CANONICALIZE_OK ("user:/tests/plugin/\\/", "", "user:/tests/plugin/\\/"); + TEST_CANONICALIZE_OK ("user:/tests/plugin/\\\\", "", "user:/tests/plugin/\\\\"); + TEST_CANONICALIZE_OK ("user:/tests/plugin/\\\\\\/", "", "user:/tests/plugin/\\\\\\/"); + TEST_CANONICALIZE_OK ("user:/tests/plugin/\\\\\\\\", "", "user:/tests/plugin/\\\\\\\\"); + TEST_CANONICALIZE_OK ("user:/tests/plugin/\\\\\\\\\\/", "", "user:/tests/plugin/\\\\\\\\\\/"); + + TEST_CANONICALIZE_OK ("user:/tests/plugin/\\//", "", "user:/tests/plugin/\\/"); + TEST_CANONICALIZE_OK ("user:/tests/plugin/\\\\/", "", "user:/tests/plugin/\\\\"); + TEST_CANONICALIZE_OK ("user:/tests/plugin/\\\\\\//", "", "user:/tests/plugin/\\\\\\/"); + TEST_CANONICALIZE_OK ("user:/tests/plugin/\\\\\\\\/", "", "user:/tests/plugin/\\\\\\\\"); + TEST_CANONICALIZE_OK ("user:/tests/plugin/\\\\\\\\\\//", "", "user:/tests/plugin/\\\\\\\\\\/"); + TEST_CANONICALIZE_OK ("/abc/%def/ghi", "", "/abc/%def/ghi"); TEST_CANONICALIZE_OK ("/abc/%d%ef%/ghi", "", "/abc/%d%ef%/ghi"); TEST_CANONICALIZE_OK ("/abc/%def%/ghi", "", "/abc/%def%/ghi"); TEST_CANONICALIZE_OK ("/abc/d%ef/ghi", "", "/abc/d%ef/ghi"); } +#undef TEST_CANONICALIZE_OK + static const char * keyNsNames[] = { "KEY_NS_NONE", "KEY_NS_CASCADING", "KEY_NS_META", "KEY_NS_SPEC", "KEY_NS_PROC", "KEY_NS_DIR", "KEY_NS_USER", "KEY_NS_SYSTEM", "KEY_NS_DEFAULT" }; @@ -675,12 +767,26 @@ static void test_unescape (void) TEST_UNESCAPE_OK ("/\\\\\\\\", KEY_NS_CASCADING, "\0\\\\"); TEST_UNESCAPE_OK ("/\\\\\\\\\\/", KEY_NS_CASCADING, "\0\\\\/"); + TEST_UNESCAPE_OK ("user:/\\/", KEY_NS_USER, "\0/"); + TEST_UNESCAPE_OK ("user:/\\\\", KEY_NS_USER, "\0\\"); + TEST_UNESCAPE_OK ("user:/\\\\\\/", KEY_NS_USER, "\0\\/"); + TEST_UNESCAPE_OK ("user:/\\\\\\\\", KEY_NS_USER, "\0\\\\"); + TEST_UNESCAPE_OK ("user:/\\\\\\\\\\/", KEY_NS_USER, "\0\\\\/"); + + TEST_UNESCAPE_OK ("user:/tests/plugin/\\/", KEY_NS_USER, "\0tests\0plugin\0/"); + TEST_UNESCAPE_OK ("user:/tests/plugin/\\\\", KEY_NS_USER, "\0tests\0plugin\0\\"); + TEST_UNESCAPE_OK ("user:/tests/plugin/\\\\\\/", KEY_NS_USER, "\0tests\0plugin\0\\/"); + TEST_UNESCAPE_OK ("user:/tests/plugin/\\\\\\\\", KEY_NS_USER, "\0tests\0plugin\0\\\\"); + TEST_UNESCAPE_OK ("user:/tests/plugin/\\\\\\\\\\/", KEY_NS_USER, "\0tests\0plugin\0\\\\/"); + TEST_UNESCAPE_OK ("/abc/%def/ghi", KEY_NS_CASCADING, "\0abc\0%def\0ghi"); TEST_UNESCAPE_OK ("/abc/%d%ef%/ghi", KEY_NS_CASCADING, "\0abc\0%d%ef%\0ghi"); TEST_UNESCAPE_OK ("/abc/%def%/ghi", KEY_NS_CASCADING, "\0abc\0%def%\0ghi"); TEST_UNESCAPE_OK ("/abc/d%ef/ghi", KEY_NS_CASCADING, "\0abc\0d%ef\0ghi"); } +#undef TEST_UNESCAPE_OK + #define TEST_ESCAPE_PART_OK(upart, part) \ do \ { \ @@ -726,6 +832,7 @@ static void test_escapePart (void) TEST_ESCAPE_PART_OK ("d\\a", "d\\\\a"); } +#undef TEST_ESCAPE_PART_OK int main (int argc, char ** argv) {