tomasilluminati commented on PR #776: URL: https://github.com/apache/commons-compress/pull/776#issuecomment-4828921548
@garydgregory Thanks, this is a great direction, I'm keen to reuse PathFence instead of carrying my own containment code. The syntactic part is a clean fit. toAbsolutePath().normalize() plus the within-root check is generic, and Compress, Configuration and Text can all share it as-is. Links are the tricky bit, and I'd lean toward composing rather than folding them in. Part of why they resist living in a path predicate is that during extraction the entry doesn't exist yet, so I can't just toRealPath() the target. The real protection ends up being operational, resolve each component no-follow, create with CREATE_NEW so an existing link is never followed, and on Linux write relative to the directory handle (SecureDirectoryStream) to close the TOCTOU window. That's bound to the act of creating files, not to judging a path. Where @ppkarwasz point lands well is the already-on-disk case, like a trusted config dir. There a resolving check (toRealPath then the same within-root test) makes sense, and that could be an optional mode in PathFence. The extractor would lean on the syntactic decision and keep its no-follow creation layer on top. So my vote is composed. PathFence owns the containment decision, extraction-specific link safety sits around it, and PathFence stays light enough for Configuration to reuse. Happy to dig into the IO PR with you and Piotr whenever it's a good time, and to bring over the link cases I already cover as tests. Thanks. -- 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]
