Doris-Breakwater commented on issue #66024:
URL: https://github.com/apache/doris/issues/66024#issuecomment-5071528462
## Breakwater initial analysis
**Verdict: confirmed, actionable authorization bug.** This is a
deterministic control-flow error in the v1 FE HTTP handler, not a hypothesis
that needs logs or a Doris profile to validate.
### What the code confirms
- On the checked master (`27cb451229c`), `MetaInfoAction.getAllDatabases()`
obtains every name from `catalog.getDbNames()`, correctly evaluates `SHOW` for
each name, and appends allowed names to `dbNameSet`. It then ignores that
result: sorting, pagination sizing, and the returned `subList` all use the
original unfiltered `dbNames`.
- `checkWithCookie(..., false)` does **not** mean unauthenticated access.
`BaseController.checkWithCookie` still validates either the `Authorization`
header or a `PALO_SESSION_ID` session and installs the authenticated identity
in `ConnectContext`; the boolean controls the additional admin/node check.
`Config.enable_all_http_auth` defaults to `false`.
- With that default, a valid restricted user can therefore discover database
names for which `SHOW` fails. With `enable_all_http_auth=true`, this handler
additionally calls `checkAdminAuth`, so restricted users are rejected before
enumeration.
- The scoped disclosure is database-name existence through this method. The
v1 table-list and schema paths have separate table privilege checks.
- Current history supports the report's attribution: blame shows the
filtered-list construction and the unfiltered sort/return have coexisted since
the original endpoint implementation in 2020. The mutable-copy adjustment
discussed in PR #65126 only prevents sorting an immutable source list and does
not create this privilege bypass.
- This is in scope under the repository threat model: a restricted
authenticated user is an in-scope actor, and discovering objects outside grant
scope is explicitly an RBAC violation. Severity/CVE handling should still be
decided through the project's security process.
### Important fix detail
The v2 implementation is a correct reference for **using the filtered
list**, but it should not be copied literally for catalog authorization. Both
v1 and v2 currently call:
```java
checkDbPriv(..., InternalCatalog.INTERNAL_CATALOG_NAME, db,
PrivPredicate.SHOW)
```
even when `{ns}` names an external catalog. Since
`AccessControllerManager.checkDbPriv` selects/checks authorization using the
supplied catalog, this can check grants for an internal database with the same
name instead of the requested external catalog. That can produce both false
denial and cross-catalog name disclosure.
The v1 fix should therefore:
1. Normalize `default_cluster` to `InternalCatalog.INTERNAL_CATALOG_NAME`
once as `catalogName`.
2. Use `catalogName` for both catalog lookup and `checkDbPriv`.
3. Collect into a clearly named `filteredDbNames`, sort that new list,
compute pagination from `filteredDbNames.size()`, and return its `subList`.
4. Avoid sorting the collection returned by `catalog.getDbNames()`; this
also makes the fix compatible with immutable name snapshots without an
otherwise unnecessary mutable copy.
The same hard-coded catalog argument should be audited in
`MetaInfoActionV2.getAllDatabases()` and `MetaInfoActionV2.getTables()` as
follow-up/same-fix scope.
### Tests recommended before merge
- With `enable_all_http_auth=false`, authenticate as a non-admin user that
has `SHOW` on one database but not another; the v1 endpoint must return only
the allowed database.
- A user with no applicable database privilege must receive an empty list.
- Exercise `limit` and `offset` with hidden names before/between visible
names to prove pagination is applied after filtering and cannot reveal or
distort results.
- Cover both `default_cluster`/internal-catalog normalization and an
external catalog, verifying that the requested catalog name is passed to
authorization.
- Preserve the `enable_all_http_auth=true` admin-only behavior.
- Add a focused regression for the equivalent hard-coded-catalog issue in v2
if it is fixed in the same change.
### Missing information / maintainer follow-up
- The issue currently has no labels, type, assignee, milestone, or linked
fix. Please add the appropriate FE/HTTP bug and security-triage metadata
according to project convention.
- The exact supported-release/backport matrix is not stated. The bug is
present on checked master and is historically old, so supported release
branches containing this endpoint should be inspected rather than assuming only
master is affected.
- A black-box reproduction transcript is absent, but it is not blocking
confirmation because the source path is conclusive. It would still be useful as
the basis of the regression test.
- No logs, runtime profile, or additional environment details are needed for
root-cause confirmation. If external catalogs are intentionally unsupported by
this v1 database-list route, that contract should be enforced explicitly;
otherwise the catalog-aware privilege check above is required.
Please route the security-sensitive RBAC bypass according to `SECURITY.md`
while coordinating the public issue.
Breakwater-GitHub-Analysis-Slot: slot_e15af872105b
--
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]
