From 8f60b04da47ffefe0e52bda2440134b42874eb75 Mon Sep 17 00:00:00 2001
From: Akhilesh Arora <akhildawra@gmail.com>
Date: Mon, 25 May 2026 15:07:37 +0200
Subject: [PATCH] proxy: split password hack payload on first separator
 (#12899)

## Problem

`PasswordHackPayload::parse` iterated through `[";", "$"]` and used
whichever separator appeared in the bytes first by *iteration* order,
not by *position* in the input. The result: a client that picked `$` as
its separator and happened to put `;` somewhere in the password got
split at the `;` instead.

Concretely, `endpoint=foobar$pass;with;semis` parsed to:

- endpoint: `foobar$pass` (rejected later by `parse_endpoint_param`, or
routed to the wrong endpoint if the prefix happened to look valid)
  - password: `with;semis`

Endpoint names are restricted to alphanumeric plus hyphen (see
`parse_endpoint_param`), so neither separator can ever appear inside an
endpoint. Whichever separator appears first in the bytes is the correct
one.

## Summary of changes

* `proxy/src/auth/password_hack.rs`: scan the input for the first
occurrence of either `;` or `$` (`bytes.iter().position(...)`), split
there, and use everything past it as the password.
* Add `parse_uses_first_separator` test covering both directions
(`$`-separated payload with `;` in the password, and the mirror case).
* Existing `parse_password_hack_payload_{project,endpoint,dollar}` tests
continue to pass.

Before:

$ cargo test -p proxy --lib
auth::password_hack::tests::parse_uses_first_separator
test auth::password_hack::tests::parse_uses_first_separator ... FAILED
    thread '...' panicked at proxy/src/auth/password_hack.rs:126:9:
    assertion `left == right` failed
      left: EndpointId("foobar$pass")
     right: "foobar"

After:

    $ cargo test -p proxy --lib auth::password_hack
    running 5 tests
    test auth::password_hack::tests::parse_uses_first_separator ... ok
test auth::password_hack::tests::parse_password_hack_payload_dollar ...
ok
test auth::password_hack::tests::parse_password_hack_payload_project ...
ok
test auth::password_hack::tests::parse_password_hack_payload_endpoint
... ok
    test auth::password_hack::tests::parse_endpoint_param_fn ... ok
    test result: ok. 5 passed; 0 failed
---
 proxy/src/auth/password_hack.rs | 42 ++++++++++++++++++++++++---------
 1 file changed, 31 insertions(+), 11 deletions(-)

diff --git a/proxy/src/auth/password_hack.rs b/proxy/src/auth/password_hack.rs
index b934c28a78..5258e97002 100644
--- a/proxy/src/auth/password_hack.rs
+++ b/proxy/src/auth/password_hack.rs
@@ -15,18 +15,18 @@ pub(crate) struct PasswordHackPayload {
 impl PasswordHackPayload {
     pub(crate) fn parse(bytes: &[u8]) -> Option<Self> {
         // The format is `project=<utf-8>;<password-bytes>` or `project=<utf-8>$<password-bytes>`.
-        let separators = [";", "$"];
-        for sep in separators {
-            if let Some((endpoint, password)) = bytes.split_once_str(sep) {
-                let endpoint = endpoint.to_str().ok()?;
-                return Some(Self {
-                    endpoint: parse_endpoint_param(endpoint)?.into(),
-                    password: password.to_owned(),
-                });
-            }
-        }
+        // The endpoint name is restricted to alphanumeric/hyphen, so it never
+        // contains either separator; split on whichever one appears first so
+        // we don't truncate the password when it contains the other separator.
+        let split = bytes.iter().position(|&b| b == b';' || b == b'$')?;
+        let (endpoint, rest) = bytes.split_at(split);
+        let password = &rest[1..];
 
-        None
+        let endpoint = endpoint.to_str().ok()?;
+        Some(Self {
+            endpoint: parse_endpoint_param(endpoint)?.into(),
+            password: password.to_owned(),
+        })
     }
 }
 
@@ -118,4 +118,24 @@ mod tests {
         assert_eq!(payload.endpoint, "foobar");
         assert_eq!(payload.password, b"pass$word");
     }
+
+    // Whichever separator the client used must win, regardless of what
+    // characters happen to appear later in the password. Previously the parser
+    // always tried `;` before `$`, which truncated the endpoint when a client
+    // used `$` as the separator but had `;` in the password (e.g. AWS DMS
+    // forbids `: ;+%` only in the password text it builds itself, but other
+    // tooling may legitimately pass these through unencoded).
+    #[test]
+    fn parse_uses_first_separator() {
+        let bytes = b"endpoint=foobar$pass;with;semis";
+        let payload = PasswordHackPayload::parse(bytes).expect("parsing failed");
+        assert_eq!(payload.endpoint, "foobar");
+        assert_eq!(payload.password, b"pass;with;semis");
+
+        // And the mirror case: `;` is the separator, password contains `$`.
+        let bytes = b"endpoint=foobar;pass$with$dollars";
+        let payload = PasswordHackPayload::parse(bytes).expect("parsing failed");
+        assert_eq!(payload.endpoint, "foobar");
+        assert_eq!(payload.password, b"pass$with$dollars");
+    }
 }