Skip to content

fix(security): harden symlink target validation during extraction#371

Merged
deepin-bot[bot] merged 1 commit intolinuxdeepin:release/eaglefrom
LiHua000:release/eagle
Mar 23, 2026
Merged

fix(security): harden symlink target validation during extraction#371
deepin-bot[bot] merged 1 commit intolinuxdeepin:release/eaglefrom
LiHua000:release/eagle

Conversation

@LiHua000
Copy link
Copy Markdown
Contributor

@LiHua000 LiHua000 commented Mar 23, 2026

Resolve symlink escape checks against real filesystem paths by validating the first existing path component with canonical resolution. This closes a bypass where lexical path checks could be fooled by pre-existing symlinks while keeping extraction behavior and performance stable.

log: fix cnnvd

Bug: https://pms.uniontech.com/bug-view-353989.html https://pms.uniontech.com/bug-view-353985.html

Summary by Sourcery

Harden archive extraction to prevent path traversal and symlink-based escapes from the target directory across all supported backends.

Bug Fixes:

  • Validate extracted file destinations against the extraction root using canonical filesystem paths to block traversal and symlink escape bugs.
  • Normalize and sanitize ZIP entry paths (dot-dot segments and backslashes) before extraction to avoid writing outside the target directory.
  • Enforce that symlink targets inside ZIP archives resolve within the extraction root before creating them.
  • Enable libarchive security flags to reject unsafe symlinks and absolute paths during extraction.

Enhancements:

  • Introduce shared helper functions to centrally validate extraction destinations and symlink targets against the extraction root.

@sourcery-ai
Copy link
Copy Markdown

sourcery-ai bot commented Mar 23, 2026

Reviewer's Guide

Hardens archive extraction security by canonicalizing paths and validating both extracted file locations and symlink targets against the real filesystem under the configured extraction root, and by enabling stricter libarchive security flags.

Sequence diagram for LibminizipPlugin extraction with hardened path checks

sequenceDiagram
    actor Caller
    participant LibminizipPlugin
    participant ExtractionSecurityHelpers

    Caller->>LibminizipPlugin: extractEntry(zipfile, file_info, options)
    LibminizipPlugin->>LibminizipPlugin: normalize strFileName (strip ../, replace \)
    LibminizipPlugin->>LibminizipPlugin: build strDestFileName
    LibminizipPlugin->>LibminizipPlugin: compute cleanTargetPath, cleanDestPath
    LibminizipPlugin-->>Caller: ET_FileWriteError (if cleanDestPath outside cleanTargetPath)

    LibminizipPlugin->>ExtractionSecurityHelpers: extractPathIsWithinTarget(options.strTargetPath, strDestFileName)
    ExtractionSecurityHelpers-->>LibminizipPlugin: allowed / rejected
    LibminizipPlugin-->>Caller: ET_FileWriteError (if rejected)

    LibminizipPlugin->>LibminizipPlugin: continue with file creation and data extraction
Loading

Sequence diagram for LibzipPlugin symlink creation with target validation

sequenceDiagram
    actor Caller
    participant LibzipPlugin
    participant ExtractionSecurityHelpers
    participant zipFile

    Caller->>LibzipPlugin: extractEntry(archive, index, options)
    LibzipPlugin->>LibzipPlugin: compute strDestFileName under options.strTargetPath
    LibzipPlugin->>ExtractionSecurityHelpers: extractPathIsWithinTarget(options.strTargetPath, strDestFileName)
    ExtractionSecurityHelpers-->>LibzipPlugin: allowed / rejected
    LibzipPlugin-->>Caller: ET_FileWriteError (if rejected)

    LibzipPlugin->>zipFile: zip_fopen / zip_fread (symlink contents)
    zipFile-->>LibzipPlugin: strBuf (symlinkTarget)

    LibzipPlugin->>ExtractionSecurityHelpers: symlinkTargetIsWithinTarget(options.strTargetPath, strDestFileName, strBuf)
    ExtractionSecurityHelpers-->>LibzipPlugin: allowed / rejected
    LibzipPlugin-->>Caller: ET_FileWriteError (if rejected)

    LibzipPlugin->>LibzipPlugin: QFile::link(strBuf, strDestFileName) (if allowed)
Loading

Class diagram for updated extraction security helpers and plugin usage

classDiagram
    class LibminizipPlugin {
        +ErrorType extractEntry(unzFile zipfile, unz_file_info file_info, const ExtractOptions &options)
    }

    class LibzipPlugin {
        +ErrorType extractEntry(zip_t *archive, zip_int64_t index, const ExtractOptions &options)
    }

    class LibarchivePlugin {
        +int extractionFlags() const
    }

    class ExtractionSecurityHelpers {
        +bool extractPathIsWithinTarget(QString extractRoot, QString absoluteDestPath)
        +bool symlinkTargetIsWithinTarget(QString extractRoot, QString symlinkFilePath, QString symlinkTarget)
    }

    LibminizipPlugin ..> ExtractionSecurityHelpers : uses
    LibzipPlugin ..> ExtractionSecurityHelpers : uses

    LibarchivePlugin ..> ExtractionSecurityHelpers : strengthened_flags
Loading

File-Level Changes

Change Details Files
Introduce shared helpers to validate that extracted paths and symlink targets remain within the extraction root using canonical filesystem resolution.
  • Add a helper that resolves the first existing component of a destination path canonically and rejects it if it falls outside the extraction root after following symlinks.
  • Add a helper that resolves symlink targets relative to the symlink’s parent directory, walks up the path until an existing component is found, and ensures the canonical target remains within the extraction root, with a lexical fallback when no components exist.
  • Declare the new helpers in the common interface header with documentation comments describing their security purpose.
3rdparty/interface/common.cpp
3rdparty/interface/common.h
Harden minizip-based extraction against path traversal and symlink-based escapes from the extraction root.
  • Normalize incoming zip entry names by stripping any "../" components and converting backslashes to the platform directory separator before building destination paths.
  • Clean and absolutize both target root and destination paths, rejecting any extraction whose normalized destination does not lie under the normalized target root.
  • Invoke the new canonical path helper to reject paths that escape the extraction root due to existing filesystem symlinks, logging diagnostic messages and returning a write error on rejection.
3rdparty/libminizipplugin/libminizipplugin.cpp
Harden libzip-based extraction by validating extracted file paths and symlink targets against the extraction root.
  • Use the new canonical path helper to verify that each destination path remains within the extraction root before creating the output file.
  • Before creating a symlink from stored content, validate the symlink target against the extraction root using the new symlink helper and reject invalid targets, emitting logging and error signals on failure.
3rdparty/libzipplugin/libzipplugin.cpp
Tighten libarchive extraction flags to leverage built-in protections against unsafe paths and symlinks.
  • Enable libarchive flags that prevent extraction through symlinks and disallow absolute paths while preserving existing dot-dot protections.
3rdparty/libarchive/libarchive/libarchiveplugin.cpp

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

Copy link
Copy Markdown

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've left some high level feedback:

  • The ../ stripping in LibminizipPlugin::extractEntry is a naive substring replacement that can mangle legitimate names containing "../" and doesn’t operate on path components; consider normalizing via QDir::cleanPath or explicit component-wise filtering of ".." segments instead.
  • extractPathIsWithinTarget relies on canonicalFilePath() of extractRoot and returns false when the root doesn’t exist yet, which may inadvertently break extracting into a not-yet-created directory; if that’s a valid use case, ensure the root directory is created/validated earlier or handle a non-existent root more explicitly.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- The `../` stripping in `LibminizipPlugin::extractEntry` is a naive substring replacement that can mangle legitimate names containing `"../"` and doesn’t operate on path components; consider normalizing via `QDir::cleanPath` or explicit component-wise filtering of `".."` segments instead.
- `extractPathIsWithinTarget` relies on `canonicalFilePath()` of `extractRoot` and returns `false` when the root doesn’t exist yet, which may inadvertently break extracting into a not-yet-created directory; if that’s a valid use case, ensure the root directory is created/validated earlier or handle a non-existent root more explicitly.

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Resolve symlink escape checks against real filesystem paths by validating the first existing path component with canonical resolution. This closes a bypass where lexical path checks could be fooled by pre-existing symlinks while keeping extraction behavior and performance stable.

log: fix cnnvd

Bug: https://pms.uniontech.com/bug-view-353989.html
https://pms.uniontech.com/bug-view-353985.html
@deepin-ci-robot
Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: LiHua000, max-lvs

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@LiHua000
Copy link
Copy Markdown
Contributor Author

/merge

@deepin-bot deepin-bot bot merged commit 94f0d00 into linuxdeepin:release/eagle Mar 23, 2026
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants