Skip to content

ed25519: Invalid out of bound reads

Low
cgwalters published GHSA-gqf4-p3gv-g8vw Jul 22, 2022

Package

ostree (ostreedev)

Affected versions

<= 2022.4

Patched versions

2022.5

Description

Impact

I (Demi Marie Obenour) have found what I believe are two security vulnerabilities in libostree. Both are low severity, and I believe the practical impact is denial of service only. Both vulnerabilities are in src/libostree/ostree-sign-ed25519.c:ostree_sign_ed25519_data_verify
(link:

gboolean ostree_sign_ed25519_data_verify (OstreeSign *self,
GBytes *data,
GVariant *signatures,
char **out_success_message,
GError **error)
{
g_return_val_if_fail (OSTREE_IS_SIGN (self), FALSE);
g_return_val_if_fail (data != NULL, FALSE);
OstreeSignEd25519 *sign = _ostree_sign_ed25519_get_instance_private(OSTREE_SIGN_ED25519(self));
if (!_ostree_sign_ed25519_is_initialized (sign, error))
return FALSE;
if (signatures == NULL)
return glnx_throw (error, "ed25519: commit have no signatures of my type");
if (!g_variant_is_of_type (signatures, (GVariantType *) OSTREE_SIGN_METADATA_ED25519_TYPE))
return glnx_throw (error, "ed25519: wrong type passed for verification");
#ifdef HAVE_LIBSODIUM
/* If no keys pre-loaded then,
* try to load public keys from storage(s) */
if (sign->public_keys == NULL)
{
g_autoptr (GVariantBuilder) builder = NULL;
g_autoptr (GVariant) options = NULL;
builder = g_variant_builder_new (G_VARIANT_TYPE ("a{sv}"));
options = g_variant_builder_end (builder);
if (!ostree_sign_ed25519_load_pk (self, options, error))
return FALSE;
}
g_debug ("verify: data hash = 0x%x", g_bytes_hash(data));
g_autoptr(GString) invalid_signatures = NULL;
guint n_invalid_signatures = 0;
for (gsize i = 0; i < g_variant_n_children(signatures); i++)
{
g_autoptr (GVariant) child = g_variant_get_child_value (signatures, i);
g_autoptr (GBytes) signature = g_variant_get_data_as_bytes(child);
g_autofree char * hex = g_malloc0 (crypto_sign_PUBLICKEYBYTES*2 + 1);
g_debug("Read signature %d: %s", (gint)i, g_variant_print(child, TRUE));
for (GList *public_key = sign->public_keys;
public_key != NULL;
public_key = public_key->next)
{
/* TODO: use non-list for tons of revoked keys? */
if (g_list_find_custom (sign->revoked_keys, public_key->data, _compare_ed25519_keys) != NULL)
{
g_debug("Skip revoked key '%s'",
sodium_bin2hex (hex, crypto_sign_PUBLICKEYBYTES*2+1, public_key->data, crypto_sign_PUBLICKEYBYTES));
continue;
}
if (crypto_sign_verify_detached ((guchar *) g_variant_get_data (child),
g_bytes_get_data (data, NULL),
g_bytes_get_size (data),
public_key->data) != 0)
{
/* Incorrect signature! */
if (invalid_signatures == NULL)
invalid_signatures = g_string_new ("");
else
g_string_append (invalid_signatures, "; ");
n_invalid_signatures++;
g_string_append_printf (invalid_signatures, "key '%s'",
sodium_bin2hex (hex, crypto_sign_PUBLICKEYBYTES*2+1, public_key->data, crypto_sign_PUBLICKEYBYTES));
}
else
{
if (out_success_message)
{
*out_success_message =
g_strdup_printf ("ed25519: Signature verified successfully with key '%s'",
sodium_bin2hex (hex, crypto_sign_PUBLICKEYBYTES*2+1, public_key->data, crypto_sign_PUBLICKEYBYTES));
}
return TRUE;
}
}
}
if (invalid_signatures)
{
g_assert_cmpuint (n_invalid_signatures, >, 0);
/* The test suite has a key ring with 100 keys. This seems insane, let's
* cap a reasonable error message at 3.
*/
if (n_invalid_signatures > 3)
return glnx_throw (error, "ed25519: Signature couldn't be verified; tried %u keys", n_invalid_signatures);
return glnx_throw (error, "ed25519: Signature couldn't be verified with: %s", invalid_signatures->str);
}
return glnx_throw (error, "ed25519: no signatures found");
#endif /* HAVE_LIBSODIUM */
return FALSE;
}
).

The first is that the ed25519 signature verification code does not check that the signature is the correct length. As a result, if the signature is too short, libsodium will end up reading a few bytes out of bounds. Theoretically, this could cause an bad signature to be accepted. However, for this to happen, the out-of-bounds data would need to form a valid signature when combined with the actual data, which means that the file must have actually been signed. A more likely possibility is that libsodium winds up reading into an adjacent unmapped page and crashes.

The second is that if there are N keys in the keyring, M revoked keys, and O signatures, libostree will consume O(N * M * O) (cubic) time checking for key revocation. This can be reduced to O(N * O + M log M + N) time or better by using an associative array to filter out revoked keys and by moving the revocation check out of the for-each-signature loop.

Patches

83e6357

Workarounds

None.

Credits

This issue was reported by Demi Marie Obenour demiobenour@gmail.com.

Severity

Low

CVE ID

No known CVE

Weaknesses

Credits