zeroshade commented on code in PR #3845: URL: https://github.com/apache/iceberg-python/pull/3845#discussion_r3873507004
########## SECURITY-THREAT-MODEL.md: ########## @@ -65,15 +65,24 @@ Iceberg Python should: already trusted with them - avoid creating new unauthorized capabilities in Iceberg Python-owned components -- avoid violating trust boundaries that Iceberg Python itself owns, such as - leaking auth, transport, or credential-bearing state across catalog or - client boundaries in the same process +- avoid violating trust boundaries that Iceberg Python itself documents as its + own Iceberg Python does not aim to be the primary enforcement point for: - user-to-user authorization inside the embedding application - storage-level authorization - service-side credential scoping performed by an external catalog +- isolation between tenants, principals, or catalogs sharing a single process + +That last point is worth stating plainly, because it is the one most often +assumed. Iceberg Python does not guarantee that state is partitioned between +catalog instances, sessions, or tenants within one interpreter. Caches, client +objects, and auth state may be process-wide. A deployment that serves multiple +principals from one process and needs them isolated must provide that +isolation itself — by process, interpreter, or another boundary it controls. +Reports that a process-wide structure is shared across catalogs or tenants are +accepted as bugs worth fixing, not as vulnerabilities. Review Comment: We might want to narrow this a little bit, as written it could excuse a genuine credential leak caused by a process-global cache or incorrectly keyed singleton. Maybe instead we word it more like: > Iceberg Python does not provide general tenant isolation for objects an application intentionally shares. However, credential-bearing state created and managed internally for independently constructed catalog or client instances must not cross those boundaries. e.g. pyiceberg owns the secret separation between independently constructed catalog or client instances. Shared non-secret state is correctness, but shared credentials, tokens, signed requests, or auth state reaching another principal remains security-relevant. ########## SECURITY-THREAT-MODEL.md: ########## @@ -223,6 +327,17 @@ A finding should be downgraded or rejected by default if it instead depends primarily on: Review Comment: I think this might not be great for automated tooling and might end up suppressing unusual but valid reports. Maybe we reword this more like: > These conditions lower confidence, but do not automatically accept or reject a report. Human review is required when finding involves credentials, outbound routing, native memory, destructive operations, or an explicitly documented security boundary. Rather than just "downgraded or rejected by default" ########## SECURITY-THREAT-MODEL.md: ########## Review Comment: Should we add links to the Iceberg security model (https://iceberg.apache.org/security/), the ASF security process (https://www.apache.org/security/) and the [email protected]/[email protected] reporting addresses? The document should state explicitly that this is triage guidance rather than vulnerability acceptance policy ########## SECURITY-THREAT-MODEL.md: ########## @@ -208,6 +224,94 @@ service are usually outside Iceberg Python's primary security boundary. If the actor already has a legitimate capability that can cause the same harm, the new path is usually not a security issue. +### 5. Reports requiring an authorized table writer or maintainer + +A principal who can already write or replace table metadata, set table +properties, or write and delete table data is operating inside the capability +set described under *Table writer or maintainer*. A report that only shows a +new route to an effect that principal can already produce legitimately is not +a security vulnerability in Iceberg Python. + +This covers, by default: + +- table properties that change client behavior for readers of that same table +- metadata a writer is already entitled to replace wholesale +- data or delete files a writer is already entitled to add + +Containment of what a writer commits — that manifest entries and write paths +stay inside the table's location — is enforced by the catalog, not by this +client. A report showing that Iceberg Python follows a path the catalog +accepted is a hardening opportunity here and a containment question there. + +### 6. Resource exhaustion, allocation amplification, and algorithmic complexity + +Availability-only findings are not treated as security vulnerabilities in +Iceberg Python by default. This includes: + +- out-of-memory conditions from attacker-influenced sizes or counts +- allocation amplification, where a small input drives a large allocation +- decompression bombs and unbounded decode output +- superlinear or exponential algorithmic complexity on attacker-influenced + input + +Iceberg Python is a client library; the surrounding service owns request +admission, resource limits, and process isolation. These reports are accepted +as robustness and hardening work, not as vulnerabilities, unless they also +demonstrate a confidentiality or integrity consequence. + +### 7. Out-of-bounds access and native memory-safety findings + +Bounds-checking gaps in native or compiled decode paths, including the Cython +Avro decoders, are treated by default as parser hardening rather than as +security vulnerabilities. Crashes, aborts, and read overruns that terminate the +process fall under this rule. Review Comment: What about demonstrated memory observability, use-after-free, attacker controlled execution? These should probably be considered security-relevant. I'd argue that native out-of-bounds access without demonstrated exploitability still requires human review rather than automatic rejection. We shouldn't dismiss a read overrun just because the report hasn't yet shown which bytes are obseravable -- 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]
