tomasilluminati commented on PR #776:
URL: https://github.com/apache/commons-compress/pull/776#issuecomment-4845573293

   @Marcono1234 thanks, both are good calls.
   
   On ALLOW_WITHIN_ROOT, you're right. The lexical normalize() check can't 
model how the OS resolves .. after a symlink component, and with the 
forward-reference ordering even a real-path check at creation wouldn't catch 
it, since the second link doesn't exist yet when the first is validated. So 
that mode can't promise every created link stays inside the root once they are 
followed together. I'd document it as best-effort and lexical, calling out that 
it cannot prevent escapes via chained or indirect symlinks, with REJECT (the 
default) or SKIP as the safe choices for untrusted archives. Worth noting that 
extraction itself never follows these links (the write path resolves every 
component no-follow), so nothing is written outside the root during extraction. 
The hazard is a created link that escapes when something follows it later, and 
the docs should say so plainly. If a hard guarantee is ever needed, the only 
way I see is a post-extraction pass that toRealPath-validates every created
  symlink and drops the escapers, which could be a future opt-in.
   
   On resolveWithinRoot and resolved.equals(rootDirectory), agreed, better to 
be explicit than to rely on the downstream ops being safe today. Entries that 
resolve to the root itself (/, ., a/.. and friends) carry nothing to 
materialize, so I'd skip them rather than target the root path. I'll have 
resolveWithinRoot signal that case and let the caller skip, so a benign / entry 
doesn't fail the extraction and nothing ever writes to or replaces the root, 
plus tests for those names.
   
   Before I push this, @garydgregory @ppkarwasz does this direction sound right 
to you (document ALLOW_WITHIN_ROOT as best-effort, and skip entries that 
resolve to the root)? If you are good with it, I'll proceed.


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

Reply via email to