core: fix hashing collisions in streaming token by including NULL ter…

…minators Fixes #4110 Added tests to detect this issue as well as changed existing tests so they with the newly generated hashes
ElektraInitiative · Dec 1, 2021 · b17c8c2 · b17c8c2
1 parent 3d7c47a
commit b17c8c2
Show file tree

Hide file tree

Showing 12 changed files with 48 additions and 15 deletions.
diff --git a/doc/news/_preparation_next_release.md b/doc/news/_preparation_next_release.md
@@ -92,6 +92,7 @@ The text below summarizes updates to the [C (and C++)-based libraries](https://w
 - Reorder `Key` and `KeySet` struct members to aviod padding and make space for a new `uint16_t` member, reserved for future use. _(Mihael Pranjić)_
 - Improve `keyReplacePrefix` by using new `keyCopy` function instead of manually copying the name of the `Key` _(@lawli3t)_
 - Added else error to core for elektraGetCheckUpdateNeeded _(Aydan Ghazani @4ydan)_
+- Include NULL terminators in hashing to avoid collisions _(@lawli3t)_
 
 ### <<Library1>>
 

diff --git a/src/libs/ease/hash.c b/src/libs/ease/hash.c
@@ -80,18 +80,21 @@ kdb_boolean_t calculateSpecificationToken (char hash_string[65], KeySet * ks, Ke
 			continue;
 		}
 
-		// Feed key name into sha_256_write() without NULL terminator (hence -1).
-		// This makes it easier to compare expected results with other sha256 tools.
-		sha_256_write (&sha_256, keyName (currentKey), keyGetNameSize (currentKey) - 1);
+		/**
+		 * Include NULL teminator in this and all following key/value strings, to avoid the following bug:
+		 * https://github.com/ElektraInitiative/libelektra/issues/4110
+		 */
+
+		sha_256_write (&sha_256, keyName (currentKey), keyGetNameSize (currentKey));
 		// Note: The value of the key itself is not relevant / part of specification. Only the key's name + its metadata!
 
 		KeySet * currentMetaKeys = keyMeta (currentKey);
 		// Feed name + values from meta keys into sha_256_write().
 		for (elektraCursor metaIt = 0; metaIt < ksGetSize (currentMetaKeys); metaIt++)
 		{
 			Key * currentMetaKey = ksAtCursor (currentMetaKeys, metaIt);
-			sha_256_write (&sha_256, keyName (currentMetaKey), keyGetNameSize (currentMetaKey) - 1);
-			sha_256_write (&sha_256, keyString (currentMetaKey), keyGetValueSize (currentMetaKey) - 1);
+			sha_256_write (&sha_256, keyName (currentMetaKey), keyGetNameSize (currentMetaKey));
+			sha_256_write (&sha_256, keyString (currentMetaKey), keyGetValueSize (currentMetaKey));
 		}
 	}
 

diff --git a/tests/ctest/test_hash.c b/tests/ctest/test_hash.c
@@ -23,7 +23,7 @@ static void test_keySet (void)
 
 	calculateSpecificationToken (hash_string, ks, parentKey);
 
-	const char * expected = "495c901c07beb0aedd636a4d20390f503cb5a4f5af2f69d32995804059867403";
+	const char * expected = "fac198709454d05176c4599290050c7a4240f265ae87304cf5d5ec3a074bb293";
 	succeed_if_fmt (strcmp (hash_string, expected) == 0, "Calculated token %s did not match expected result %s.", hash_string,
 			expected);
 
@@ -65,6 +65,34 @@ static void test_onlyKeysBelowParentKey (void)
 	ksDel (ksWithKeysFromTwoApps);
 }
 
+/**
+ * Test whether streaming API is aware of character order.
+ */
+static void test_hashesMetadata (void)
+{
+	char hash_string_1[65];
+	char hash_string_2[65];
+	KeySet *ks1, *ks2;
+	Key *key1, *key2;
+
+	key1 = keyNew ("/sw/application/myapp/#0/current", KEY_META, "aa", "bb", KEY_END);
+	key2 = keyNew ("/sw/application/myapp/#0/current", KEY_META, "a", "abb", KEY_END);
+
+	ks1 = ksNew (1, key1, KS_END);
+	ks2 = ksNew (1, key2, KS_END);
+
+	succeed_if (calculateSpecificationToken (hash_string_1, ks1, key1), "Could not calculate specification token");
+	succeed_if (calculateSpecificationToken (hash_string_2, ks2, key2), "Could not calculate specification token");
+
+	succeed_if (strcmp (hash_string_1, hash_string_2) != 0,
+		    "Specification Tokens of Keys with different Meta KeySets should be different");
+
+	keyDel (key1);
+	keyDel (key1);
+	ksDel (ks1);
+	ksDel (ks2);
+}
+
 int main (int argc, char ** argv)
 {
 	printf ("HASH    TESTS\n");
@@ -74,6 +102,7 @@ int main (int argc, char ** argv)
 
 	test_keySet ();
 	test_onlyKeysBelowParentKey ();
+	test_hashesMetadata ();
 
 	printf ("\ntest_hash RESULTS: %d test(s) done. %d error(s).\n", nbTest, nbError);
 

diff --git a/tests/shell/gen/highlevel/commands.expected.c b/tests/shell/gen/highlevel/commands.expected.c
@@ -106,7 +106,7 @@ int loadConfiguration (Elektra ** elektra,
 
 	KeySet * contract = ksNew (4,
 	keyNew ("system:/elektra/contract/highlevel/check/spec/mounted", KEY_VALUE, "1", KEY_END),
-	keyNew ("system:/elektra/contract/highlevel/check/spec/token", KEY_VALUE, "e5e0c78ce9d88840d87ee32b9c9b2b6fe859231ea0b516fd9f3f40bec4b27d0a", KEY_END),
+	keyNew ("system:/elektra/contract/highlevel/check/spec/token", KEY_VALUE, "a3e7c9f96b1a50a3b875296745f3d94d661ccc9d43d6587b11dbb4a021931133", KEY_END),
 	keyNew ("system:/elektra/contract/highlevel/helpmode/ignore/require", KEY_VALUE, "1", KEY_END),
 	keyNew ("system:/elektra/contract/mountglobal/gopts", KEY_END),
 	KS_END);

diff --git a/tests/shell/gen/highlevel/empty.expected.c b/tests/shell/gen/highlevel/empty.expected.c
@@ -93,7 +93,7 @@ int loadConfiguration (Elektra ** elektra,
 
 	KeySet * contract = ksNew (4,
 	keyNew ("system:/elektra/contract/highlevel/check/spec/mounted", KEY_VALUE, "1", KEY_END),
-	keyNew ("system:/elektra/contract/highlevel/check/spec/token", KEY_VALUE, "15923d95dcd350f34919d052c8a262e284e56725db3557eef24b69368ca9d5e0", KEY_END),
+	keyNew ("system:/elektra/contract/highlevel/check/spec/token", KEY_VALUE, "57b56c1769a865c366092f028c147c6cec19717a72868a47d139f209b17b85b1", KEY_END),
 	keyNew ("system:/elektra/contract/highlevel/helpmode/ignore/require", KEY_VALUE, "1", KEY_END),
 	keyNew ("system:/elektra/contract/mountglobal/gopts", KEY_END),
 	KS_END);

diff --git a/tests/shell/gen/highlevel/enum.expected.c b/tests/shell/gen/highlevel/enum.expected.c
@@ -98,7 +98,7 @@ int loadConfiguration (Elektra ** elektra,
 
 	KeySet * contract = ksNew (4,
 	keyNew ("system:/elektra/contract/highlevel/check/spec/mounted", KEY_VALUE, "1", KEY_END),
-	keyNew ("system:/elektra/contract/highlevel/check/spec/token", KEY_VALUE, "fbb054456ff70fde7e4f184ec86eb130a99b4b3712d0c9d496e78bd262fb8c8d", KEY_END),
+	keyNew ("system:/elektra/contract/highlevel/check/spec/token", KEY_VALUE, "056823c1e2aa0271dd64537180d02010d5e89b08e5804b6b25c92ce177507322", KEY_END),
 	keyNew ("system:/elektra/contract/highlevel/helpmode/ignore/require", KEY_VALUE, "1", KEY_END),
 	keyNew ("system:/elektra/contract/mountglobal/gopts", KEY_END),
 	KS_END);

diff --git a/tests/shell/gen/highlevel/externalspec.expected.c b/tests/shell/gen/highlevel/externalspec.expected.c
@@ -88,7 +88,7 @@ int loadConfiguration (Elektra ** elektra,
 
 	KeySet * contract = ksNew (4,
 	keyNew ("system:/elektra/contract/highlevel/check/spec/mounted", KEY_VALUE, "1", KEY_END),
-	keyNew ("system:/elektra/contract/highlevel/check/spec/token", KEY_VALUE, "28025ee44839c1274cfbdffc51b42e8b37b485e149168de78f52c2081fc1c1fa", KEY_END),
+	keyNew ("system:/elektra/contract/highlevel/check/spec/token", KEY_VALUE, "1f56a956f8d2714d272945529a986f5c9eeb0a00ee9211f0e5a4628ea1d02063", KEY_END),
 	keyNew ("system:/elektra/contract/highlevel/helpmode/ignore/require", KEY_VALUE, "1", KEY_END),
 	keyNew ("system:/elektra/contract/mountglobal/gopts", KEY_END),
 	KS_END);

diff --git a/tests/shell/gen/highlevel/externalwithdefaults.expected.c b/tests/shell/gen/highlevel/externalwithdefaults.expected.c
@@ -95,7 +95,7 @@ int loadConfiguration (Elektra ** elektra,
 
 	KeySet * contract = ksNew (4,
 	keyNew ("system:/elektra/contract/highlevel/check/spec/mounted", KEY_VALUE, "1", KEY_END),
-	keyNew ("system:/elektra/contract/highlevel/check/spec/token", KEY_VALUE, "97af78f96fb881d4949f8df0f61d38751fbc7c350cb7f8cc01c81f14695b66c0", KEY_END),
+	keyNew ("system:/elektra/contract/highlevel/check/spec/token", KEY_VALUE, "9d087f92337227aca94375f39f0596437e2b51f1c9ec6a26bdc326588b26353d", KEY_END),
 	keyNew ("system:/elektra/contract/highlevel/helpmode/ignore/require", KEY_VALUE, "1", KEY_END),
 	keyNew ("system:/elektra/contract/mountglobal/gopts", KEY_END),
 	KS_END);

diff --git a/tests/shell/gen/highlevel/nosetter.expected.c b/tests/shell/gen/highlevel/nosetter.expected.c
@@ -98,7 +98,7 @@ int loadConfiguration (Elektra ** elektra,
 
 	KeySet * contract = ksNew (4,
 	keyNew ("system:/elektra/contract/highlevel/check/spec/mounted", KEY_VALUE, "1", KEY_END),
-	keyNew ("system:/elektra/contract/highlevel/check/spec/token", KEY_VALUE, "a5986b1414d7bd81cf112c89ddebb4545be9c4fc331aaf7a1b49da5ddd1332ce", KEY_END),
+	keyNew ("system:/elektra/contract/highlevel/check/spec/token", KEY_VALUE, "381549fc7e2655e002bad03a749fbca628baa10290d1785b622ca16ae57a8ba9", KEY_END),
 	keyNew ("system:/elektra/contract/highlevel/helpmode/ignore/require", KEY_VALUE, "1", KEY_END),
 	keyNew ("system:/elektra/contract/mountglobal/gopts", KEY_END),
 	KS_END);

diff --git a/tests/shell/gen/highlevel/notype.expected.c b/tests/shell/gen/highlevel/notype.expected.c
@@ -94,7 +94,7 @@ int loadConfiguration (Elektra ** elektra,
 
 	KeySet * contract = ksNew (4,
 	keyNew ("system:/elektra/contract/highlevel/check/spec/mounted", KEY_VALUE, "1", KEY_END),
-	keyNew ("system:/elektra/contract/highlevel/check/spec/token", KEY_VALUE, "1cbc70fee5ca4d36d9dbaf6b37e5595f8d12ad3fa39a6778b512bd2e4b3321bf", KEY_END),
+	keyNew ("system:/elektra/contract/highlevel/check/spec/token", KEY_VALUE, "b9879003cc3bb1c92569a95d1b22b2969a33b14a700ad81ef0f73eef086cb151", KEY_END),
 	keyNew ("system:/elektra/contract/highlevel/helpmode/ignore/require", KEY_VALUE, "1", KEY_END),
 	keyNew ("system:/elektra/contract/mountglobal/gopts", KEY_END),
 	KS_END);

diff --git a/tests/shell/gen/highlevel/simple.expected.c b/tests/shell/gen/highlevel/simple.expected.c
@@ -98,7 +98,7 @@ int loadConfiguration (Elektra ** elektra,
 
 	KeySet * contract = ksNew (4,
 	keyNew ("system:/elektra/contract/highlevel/check/spec/mounted", KEY_VALUE, "1", KEY_END),
-	keyNew ("system:/elektra/contract/highlevel/check/spec/token", KEY_VALUE, "5457287d345b68df64dc9be9db323d135c412818a0209ccaee88808b1afe22a6", KEY_END),
+	keyNew ("system:/elektra/contract/highlevel/check/spec/token", KEY_VALUE, "8065d900552e6bfe4cfd402a04a8f04a28dbaeac7421aea3ff6e98bea5bed8d5", KEY_END),
 	keyNew ("system:/elektra/contract/highlevel/helpmode/ignore/require", KEY_VALUE, "1", KEY_END),
 	keyNew ("system:/elektra/contract/mountglobal/gopts", KEY_END),
 	KS_END);

diff --git a/tests/shell/gen/highlevel/struct.expected.c b/tests/shell/gen/highlevel/struct.expected.c
@@ -107,7 +107,7 @@ int loadConfiguration (Elektra ** elektra,
 
 	KeySet * contract = ksNew (4,
 	keyNew ("system:/elektra/contract/highlevel/check/spec/mounted", KEY_VALUE, "1", KEY_END),
-	keyNew ("system:/elektra/contract/highlevel/check/spec/token", KEY_VALUE, "ead719cafcfe60b9f86e10836b4bc9759f9ca581cb4ca61bcf202c57ea788a53", KEY_END),
+	keyNew ("system:/elektra/contract/highlevel/check/spec/token", KEY_VALUE, "425b0807cbeb1c0d7629d0bc03d4c0ec0fb57ae385f8c90410c0c4594830cc1c", KEY_END),
 	keyNew ("system:/elektra/contract/highlevel/helpmode/ignore/require", KEY_VALUE, "1", KEY_END),
 	keyNew ("system:/elektra/contract/mountglobal/gopts", KEY_END),
 	KS_END);