Sahana2524 opened a new pull request, #416: URL: https://github.com/apache/commons-net/pull/416
Thanks for your contribution to [Apache Commons](https://commons.apache.org/)! Your help is appreciated! Before you push a pull request, review this list: - [x] Read the [contribution guidelines](CONTRIBUTING.md) for this project. - [x] Read the [ASF Generative Tooling Guidance](https://www.apache.org/legal/generative-tooling.html) if you use Artificial Intelligence (AI). - [x] I used AI to create any part of, or all of, this pull request. Which AI tool was used to create this pull request, and to what extent did it contribute? - [x] Run a successful build using the default [Maven](https://maven.apache.org/) goal with `mvn`; that's `mvn` on the command line by itself. - [x] Write unit tests that match behavioral changes, where the tests fail if the changes to the runtime are not applied. This may not always be possible, but it is a best practice. - [x] Write a pull request description that is detailed enough to understand what the pull request does, how, and why. - [x] Each commit in the pull request should have a meaningful subject line and body. Note that a maintainer may squash commits during the merge process. Since 3.9.0 (the CVE-2021-37533 change), `_parsePassiveModeReply` ignores the address in the PASV reply by default and uses `_socket_.getInetAddress()` instead. That is correct for `FTPClient`, but in `FTPHTTPClient` the control socket is connected to the HTTP proxy, so the substituted address is the proxy itself. `_openDataConnection_` then sends: ``` CONNECT <proxy-address>:<port> HTTP/1.1 ``` where the port comes from the FTP server's `227` reply. IPv4 PASV is the default path here, since EPSV is only tried for IPv6 or with `setUseEPSVwithIPv4(true)`. Two consequences: - Plain IPv4 passive transfers through `FTPHTTPClient` go to the wrong host by default. - A malicious or compromised FTP server can pick any port on the proxy's own address, so the proxy opens a connection to whatever listens there (admin or management services that are normally only reachable from the proxy host). On `STOR` the client streams file data into that socket. This is a server-side request forgery through the proxy (CWE-918), and the FTP server should not get to choose a destination other than itself. The fix uses the host given to `connect()` (the existing `tunnelHost` field, already used for EPSV) as the PASV target, and keeps the PASV reply address only when the legacy `ipAddressFromPasvResponse` flag is set, matching what `FTPClient` does. `tunnelHost` is now set in `connect()` rather than in `tunnelHandshake()`, so a data tunnel built in legacy mode can no longer overwrite the control host that later EPSV transfers rely on. `FTPHTTPClientTest` runs a small fake proxy that tunnels the control connection, answers PASV with `(10,1,2,3,4,1)`, and records the second `CONNECT` line. Without the fix it is `CONNECT 127.0.0.1:1025` (the proxy address); with it, `CONNECT ftp.example.invalid:1025`. The default `mvn` build passes locally on JDK 21. -- 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]
