CVE-2022-4768: Improper Neutralization of Special Elements in Output Used by a Downstream Component ('Injection') (CWE-74)
value: A vulnerability was found in Dropbox merou. It has been classified as critical. Affected is the function add_public_key of the file grouper/public_key.py of the component SSH Public Key Handler. The manipulation of the argument public_key_str leads to injection. It is possible to launch the attack remotely. The name of the patch is d93087973afa26bc0a2d0a5eb5c0fde748bdd107. It is recommended to apply a patch to fix this issue. VDB-216906 is the identifier assigned to this vulnerability.
CWE description: The product constructs all or part of a command, data structure, or record using externally-influenced input from an upstream component, but it does not neutralize or incorrectly neutralizes special elements that could modify how it is parsed or interpreted when it is sent to a downstream component.
CVSS v3 base score: 9.8
Published: 2022-12-27T23:15Z
Repository: https://github.com/dropbox/merou
Language: Python
Files changed: grouper/public_key.py, tests/constants.py, tests/public_key_test.py
Commit date: 2022-03-28 18:25:44 -0500
Commit message: Block injection attack in ssh public key handling (#673)

Also needed to fix the setup a bit to address a version incompatibility
Diff: diff --git a/grouper/public_key.py b/grouper/public_key.py
index d77c600..8158ce3 100644
--- a/grouper/public_key.py
+++ b/grouper/public_key.py
@@ -70,10 +70,17 @@ def add_public_key(session, user, public_key_str):
     try:
         pubkey.parse()
     except sshpubkeys.InvalidKeyException as e:
         raise PublicKeyParseError(str(e))
 
+    # Allowing newlines can lead to injection attacks depending on how the key is
+    # consumed, such as if it's dumped in an authorized_keys file with a `command`
+    # restriction.
+    # Note parsing the key is insufficient to block this.
+    if "\r" in public_key_str or "\n" in public_key_str:
+        raise PublicKeyParseError("Public key cannot have newlines")
+
     try:
         get_plugin_proxy().will_add_public_key(pubkey)
     except PluginRejectedPublicKey as e:
         raise BadPublicKey(str(e))
 
diff --git a/tests/constants.py b/tests/constants.py
index a8566e1..6fbcea9 100644
--- a/tests/constants.py
+++ b/tests/constants.py
@@ -14,10 +14,19 @@ SSH_KEY_2 = (
     "cxCgjLj9 to/ key2"
 )
 
 SSH_KEY_BAD = "ssh-rsa AAAblahblahkey some-comment"
 
+SSH_KEY_BAD_MULTILINE = SSH_KEY_1 + "\r" + SSH_KEY_2
+
+SSH2_KEY_BAD = """\
+---- BEGIN SSH2 PUBLIC KEY ----
+foobar: this is a chance to hide bad things
+AAAAC3NzaC1lZDI1NTE5AAAAIJ5O/AXibtVhySDYn60ATXftAU1oCe4BQubFYoV2juEb
+---- END SSH2 PUBLIC KEY ----
+"""
+
 SSH_KEY_RSA_1024 = (
     "ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAAAgQDhKgSsCJR9UyQa/Gsheb5F56hg23CVnCLpmpyC2JMqVEptG9CL83Oft"
     "pOPvEb/785Act4En1VFvwMwTj25VurbG3XI984csiNdWPlM1ke4lHK2PQepSYyZVYn+hhXhzSILNDixhBYeDVv4GOfJM1"
     "HBO2AEtupOMHOqtnQLjV1UnQ== rsa-1024"
 )
diff --git a/tests/public_key_test.py b/tests/public_key_test.py
index 103e85f..9b9e3dc 100644
--- a/tests/public_key_test.py
+++ b/tests/public_key_test.py
@@ -9,11 +9,11 @@ from grouper.public_key import (
     BadPublicKey,
     DuplicateKey,
     get_public_keys_of_user,
     PublicKeyParseError,
 )
-from tests.constants import SSH_KEY_1, SSH_KEY_BAD
+from tests.constants import SSH2_KEY_BAD, SSH_KEY_1, SSH_KEY_BAD, SSH_KEY_BAD_MULTILINE
 from tests.fixtures import session, users  # noqa: F401
 
 
 class PublicKeyPlugin(BasePlugin):
     def will_add_public_key(self, key):
@@ -39,10 +39,20 @@ def test_bad_key(session, users):  # noqa: F811
         add_public_key(session, user, SSH_KEY_BAD)
 
     assert get_public_keys_of_user(session, user.id) == []
 
 
+@pytest.mark.parametrize("key", [SSH_KEY_BAD_MULTILINE, SSH2_KEY_BAD])
+def test_multiline_key(key, session, users):  # noqa: F811
+    user = users["cbguder@a.co"]
+
+    with pytest.raises(PublicKeyParseError, match="Public key cannot have newlines"):
+        add_public_key(session, user, key)
+
+    assert get_public_keys_of_user(session, user.id) == []
+
+
 @patch("grouper.public_key.get_plugin_proxy")
 def test_rejected_key(get_plugin_proxy, session, users):  # noqa: F811
     get_plugin_proxy.return_value = PluginProxy([PublicKeyPlugin()])
 
     user = users["cbguder@a.co"]

Vulnerable code: from tests.constants import SSH_KEY_1, SSH_KEY_BAD
Fixed code:     # Allowing newlines can lead to injection attacks depending on how the key is
    # consumed, such as if it's dumped in an authorized_keys file with a `command`
    # restriction.
    # Note parsing the key is insufficient to block this.
    if "\r" in public_key_str or "\n" in public_key_str:
        raise PublicKeyParseError("Public key cannot have newlines")

SSH_KEY_BAD_MULTILINE = SSH_KEY_1 + "\r" + SSH_KEY_2

SSH2_KEY_BAD = """\
---- BEGIN SSH2 PUBLIC KEY ----
foobar: this is a chance to hide bad things
AAAAC3NzaC1lZDI1NTE5AAAAIJ5O/AXibtVhySDYn60ATXftAU1oCe4BQubFYoV2juEb
---- END SSH2 PUBLIC KEY ----
"""

from tests.constants import SSH2_KEY_BAD, SSH_KEY_1, SSH_KEY_BAD, SSH_KEY_BAD_MULTILINE
@pytest.mark.parametrize("key", [SSH_KEY_BAD_MULTILINE, SSH2_KEY_BAD])
def test_multiline_key(key, session, users):  # noqa: F811
    user = users["cbguder@a.co"]

    with pytest.raises(PublicKeyParseError, match="Public key cannot have newlines"):
        add_public_key(session, user, key)

    assert get_public_keys_of_user(session, user.id) == []