tomasilluminati commented on PR #776: URL: https://github.com/apache/commons-compress/pull/776#issuecomment-4848504176
@garydgregory I'm in, happy to help with you and @ppkarwasz. On "one class or composed", I'd compose and keep PathFence a purely lexical predicate (toAbsolutePath().normalize() then startsWith, no filesystem access), so it stays trivially reusable for Configuration, Text and JEXL. One thing I'd make explicit in its contract though: lexical containment alone does not stop symlink traversal. normalize() never touches the filesystem, so a symlinked component slips straight through. It should therefore state plainly that PathFence is a syntactic gate, safe on its own only for trusted or link-free trees, and must be paired with real resolution for untrusted input. That resolution is the separate utility you mentioned. Refining my own earlier comment, I'd keep the toRealPath check in the utility rather than as a mode inside PathFence, so PathFence never touches the filesystem. It carries the two modes the cases need: - toRealPath then containment for the trusted on-disk case (the CATALINA_BASE conf symlink). It resolves links but needs the path to exist, so it fits reads, not targets that don't exist yet. - no-follow resolution for untrusted input and for creation, never following a link, which is what the Extractor already does. That puts the security-sensitive resolution in one place and keeps PathFence small enough to reuse everywhere. If that sounds right to you, I can start there. Meanwhile I'll keep #776 moving on Marcono1234's two points, neither depends on PathFence. -- 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]
