codeant-ai-for-open-source[bot] commented on code in PR #40673:
URL: https://github.com/apache/superset/pull/40673#discussion_r3415538112
##########
superset/extensions/ssh.py:
##########
@@ -15,26 +15,61 @@
# specific language governing permissions and limitations
# under the License.
+import base64
+import binascii
import logging
+import socket
from io import StringIO
from typing import TYPE_CHECKING
+import paramiko
import sshtunnel
from flask import Flask
from paramiko import RSAKey
+from paramiko.pkey import UnknownKeyType
-from superset.commands.database.ssh_tunnel.exceptions import
SSHTunnelDatabasePortError
+from superset.commands.database.ssh_tunnel.exceptions import (
+ SSHTunnelDatabasePortError,
+ SSHTunnelHostKeyVerificationError,
+)
from superset.databases.utils import make_url_safe
from superset.utils.class_utils import load_class_from_name
if TYPE_CHECKING:
from superset.databases.ssh_tunnel.models import SSHTunnel
+logger = logging.getLogger(__name__)
+
+
+def _parse_authorized_key(authorized_key: str) -> paramiko.PKey:
+ """
+ Parse a host key in authorized-key form (``"<type> <base64>[ comment]"``)
into a
+ :class:`paramiko.PKey`. The optional trailing comment field and surrounding
+ whitespace are ignored.
+
+ :raises ValueError: if the value is empty or cannot be parsed as a host
key.
+ """
+ fields = authorized_key.strip().split()
+ if len(fields) < 2:
+ raise ValueError("Host key must be in 'ssh-<type> <base64>' form")
+ key_type, key_b64 = fields[0], fields[1]
+ try:
+ key_bytes = base64.b64decode(key_b64)
Review Comment:
**Suggestion:** Base64 decoding is done in permissive mode, which can
silently ignore invalid characters and accept malformed host keys as different
byte payloads. This can cause operators to pin an unintended key value instead
of getting a hard validation error. Decode with strict validation so malformed
key material is rejected deterministically. [security]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ SSH-tunneled SQL Lab queries may accept mis-parsed host keys.
- ⚠️ Dataset introspection via SSH tunnels could pin unintended keys.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. An administrator configures an SSH tunnel in
`superset/databases/ssh_tunnel/models.py:37-80`, setting
`SSHTunnel.server_host_key`
(lines 75-79) to a value containing the correct key type plus a base64
payload with an
extra invalid character embedded in the payload.
2. A feature that uses database connections via SSH—such as SQL Lab
cancellation in
`superset/sql_lab.py:731-735`, query cancellation in
`superset/sql/execution/executor.py:1151-1156`, or dataset introspection in
`superset/datasets/datetime_format_detector.py:96-8`—calls
`database.get_sqla_engine()` in
`superset/models/core.py:7-21`.
3. Inside `Database.get_sqla_engine`,
`ssh_manager_factory.instance.create_tunnel` at
`superset/models/core.py:26-30` invokes `SSHManager.create_tunnel` in
`superset/extensions/ssh.py:167-210`, which in turn calls
`SSHManager._verify_host_key` at
`superset/extensions/ssh.py:86-125`.
4. `_verify_host_key` calls `_parse_authorized_key` at
`superset/extensions/ssh.py:44-63`;
in that helper, `base64.b64decode(key_b64)` at line 57 runs in permissive
mode and
silently discards non-base64 characters, so a malformed `server_host_key`
string with
stray characters can still decode to some byte sequence and be accepted as a
`paramiko.PKey` instead of causing a hard validation error, potentially
pinning an
unintended host key value.
```
</details>
[Fix in
Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=0ab17fce14fc45efa496d6aa2216361c&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
| [Fix in VSCode
Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=0ab17fce14fc45efa496d6aa2216361c&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/extensions/ssh.py
**Line:** 57:57
**Comment:**
*Security: Base64 decoding is done in permissive mode, which can
silently ignore invalid characters and accept malformed host keys as different
byte payloads. This can cause operators to pin an unintended key value instead
of getting a hard validation error. Decode with strict validation so malformed
key material is rejected deterministically.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40673&comment_hash=1c96646d10105205e3333767282768010cc4efb75eb59cc1479b0852dec17ac8&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40673&comment_hash=1c96646d10105205e3333767282768010cc4efb75eb59cc1479b0852dec17ac8&reaction=dislike'>👎</a>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
