CalvinKirs opened a new pull request, #65987:
URL: https://github.com/apache/doris/pull/65987

   ### What problem does this PR solve?
   
   Hardens the JDBC driver loading path and removes an unnecessary file-upload
   attack surface on the FE.
   
   Three things:
   
   1. **Structural validation of `jdbc_driver_secure_path`.** The allowed-path 
check
      used a raw string prefix (`driverUrl.startsWith(allowedPath)`), which 
could be
      bypassed by:
      - prefix confusion — an allowlist of `/opt/drivers` also matched
        `/opt/drivers-evil/x.jar`;
      - path traversal — `file:///opt/drivers/../../etc/x.jar` still passed.
      Matching is now component-based: `file://` URLs are normalized to `Path` 
and
      compared with `Path.startsWith`; `http(s)` URLs compare scheme/host/port 
and a
      component-based path prefix. A driver URL that fails to parse is now 
rejected
      (fail-closed) instead of being let through.
   
   2. **`jdbc_driver_url_white_list` is now fe.conf-only.** It was `mutable = 
true`,
      so the remote-jar allowlist could be widened at runtime via
      `ADMIN SET FRONTEND CONFIG`. It is now non-mutable and can only be set in
      `fe.conf` (takes effect after a restart).
   
   3. **Remove the file-upload HTTP endpoint.** `UploadAction`
      (`POST/PUT/GET/DELETE /api/{ns}/{db}/{tbl}/upload`) and its
      `LoadSubmitter` / `TmpFileMgr` helpers let any user with `LOAD` privilege 
write
      arbitrary files into the FE's `tmp_dir`. They are removed. This is 
unrelated to
      the CopyInto endpoint (`/copy/upload`), which is untouched.
   
   ### Behavior changes (please read)
   
   - The data-upload endpoint `/api/{ns}/{db}/{tbl}/upload` is **removed**. 
Web-UI
     pages or scripts that used it to preview/import a file will get 404. 
Mainstream
     import paths (Stream Load, Broker Load, Routine Load, INSERT, S3 Load, 
CopyInto)
     are unaffected.
   - `jdbc_driver_url_white_list` can **no longer be changed at runtime**; 
scripts
     relying on `ADMIN SET FRONTEND CONFIG` for it will now get a "not mutable" 
error.
   - `jdbc_driver_secure_path` matching is stricter: driver URLs that only 
passed by a
     coincidental string prefix or via `..` are now rejected.
   - A driver URL that cannot be parsed as a URI is now rejected instead of 
being
     accepted.
   - **`jdbc_driver_secure_path` empty-value semantics changed**: previously an 
empty
     value meant "allow all"; it now denies all scheme-qualified URLs (bare 
filenames
     under `jdbc_drivers_dir` still work). The `*` wildcard still means 
allow-all and
     is unchanged, so deployments that keep the default `*` see no change.
   
   ### Release note
   
   Removed the `/api/{ns}/{db}/{tbl}/upload` file-upload endpoint. Hardened JDBC
   driver URL validation (structural path matching, fail-closed) and made
   `jdbc_driver_url_white_list` settable only in `fe.conf`.
   
   ### Check List (For Author)
   
   - Test <!-- At least one of them must be included. -->
       - [x] Regression test
       - [x] Unit Test
       - [ ] Manual test (add detailed scripts or steps below)
       - [ ] No need to test or manual test. Explain why:
   
   - Behavior changed:
       - [ ] No.
       - [x] Yes. <!-- Described above. -->
   
   - Does this need documentation?
       - [ ] No.
       - [x] Yes. <!-- The removed endpoint and the config changes should be 
documented. -->
   


-- 
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]

Reply via email to