SEPURI-SAI-KRISHNA opened a new issue, #12594: URL: https://github.com/apache/seatunnel/issues/12594
### Search before asking - [X] I had searched in the [issues](https://github.com/apache/seatunnel/issues?q=is%3Aissue+label%3A%22bug%22) and found no similar issues. ### What happened `BasicAuthFilter` compares request credentials against the configured ones in a way whose cost depends on the secret being checked. ```java // seatunnel-engine/seatunnel-engine-server/src/main/java/org/apache/seatunnel/ // engine/server/rest/filter/BasicAuthFilter.java:87-88 if (username.equals(httpConfig.getBasicAuthUsername()) && password.equals(httpConfig.getBasicAuthPassword())) { ``` There are two independent issues on that line. `String.equals` returns as soon as two characters differ, so the work it performs grows with the number of leading characters that already match. `&&` short-circuits, so when the username does not match, the password comparison never runs at all. The filter therefore does a measurably different amount of work for a wrong username than for a correct username with a wrong password. Together these are CWE-208, an observable discrepancy in the comparison that guards the REST API and the web UI. ### SeaTunnel Version `dev` (3.0.0-SNAPSHOT), verified at `d7e9931b`. ### What you expected to happen Credential comparison that does not vary with how much of the secret was guessed, and that evaluates both halves regardless of whether the first matched. ### This is hardening, not a vulnerability I checked with ASF Security before filing anything, and they have reviewed it and concluded it is not a vulnerability. Recording that here so nobody treats this as an embargoed disclosure. Their reasoning, which I agree with: the published threat model (https://seatunnel.apache.org/security) places REST API v2 and the Web UI inside the administrative trust boundary and states that any client able to reach a management interface should be treated as a cluster administrator, with the recommendation to run SeaTunnel behind a private network or equivalent. Anyone positioned to measure the filter is therefore already where the model treats them as an administrator. There is also almost nothing to enumerate: this is one operator-configured account with a documented default, not a user database. So the case for the change is code quality rather than risk. The comparison is on an authentication path, the fix is small and local, and it does not alter which credentials are accepted. No proof of concept is attached and none was built. ### Suggested fix Compare the UTF-8 bytes with `MessageDigest.isEqual`, which accumulates the difference across bytes rather than returning at the first mismatch, and evaluate both comparisons before branching so neither is skipped. One detail worth getting right: `MessageDigest.isEqual` reads exactly as many bytes as its **first** argument holds, which is visible in the JDK bytecode. The value taken from the request should therefore be passed first, so the number of bytes read is one the caller already knows and does not track the length of the configured credential. The configured credentials are plain `String` fields and can be unset, so the replacement also needs an explicit null check. `String.equals(null)` is safely `false` today, whereas reading bytes off a null would turn a 401 into a 500. ### Are you willing to submit PR? - [X] Yes I am willing to submit a PR! ### Code of Conduct - [X] I agree to follow this project's [Code of Conduct](https://www.apache.org/foundation/policies/conduct) -- 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]
