Skip to content

Verify ZIP symlink data only after its bounds check in PureLib - #6675

Merged
AmirMS (AmelBawa-msft) merged 2 commits into
microsoft:masterfrom
AmelBawa-msft:user/amelbawa/fix-zip-symlink-bounds
Nov 12, 2026
Merged

AmirMS (AmelBawa-msft) merged 2 commits into
microsoft:masterfrom
AmelBawa-msft:user/amelbawa/fix-zip-symlink-bounds

Conversation

@AmelBawa-msft

@AmelBawa-msft AmirMS (AmelBawa-msft) commented Nov 11, 2026 •

Copy link
Copy Markdown
Contributor

📖 Description

  • Moved the symlink check in pure_zip_meta after the existing bounds check, so symlink data is only read once it's known to be within the buffer. Valid archives behave the same.
  • Added a note to pure.h and a "Local changes" section to the PureLib readme, so these changes are kept on future subtree updates.

🔗 References

🔍 Validation

✅ Checklist

🤖 AI Assistance

  • AI assistance was used and has been disclosed in this PR
  • No AI assistance was used

📋 Issue Type

  • Bug fix
  • Feature
  • Task
Microsoft Reviewers: Open in CodeFlow

@AmelBawa-msft
AmirMS (AmelBawa-msft) requested a review from a team as a code owner November 11, 2026 20:14
@AmelBawa-msft
AmirMS (AmelBawa-msft) requested a balanced review from Copilot November 11, 2026 20:17

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Direct parser regression tests are still needed to verify that truncated stored Unix symlink payloads return PURE_E_ZIP_LFH_DATA_OVERFLOW while valid symlinks succeed.

1 open issue

Review effort: Balanced

Comment thread src/PureLib/pure/pure.h Resolved
Comment thread src/PureLib/pure/pure.h
if (lfh_offset > size) return PURE_E_ZIP_LFH_DATA_OVERFLOW;
// Verify symlink data only after checking that it is within the buffer:
{
int error = pure_zip_verify_symlink(&cdh, &lfh, buffer);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Has anyone looked at submitting this as an official patch to the upstream?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good question! Upstream hasn't been active since 2020, so I went with a local fix for now.

@ranm-msft ranm-msft left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Bounds checks now run before symlink target validation. The valid-symlink and one-byte-past-end cases cover the intended behavior. Looks good.

@AmelBawa-msft
AmirMS (AmelBawa-msft) merged commit 11e5d8b into microsoft:master Nov 12, 2026
9 checks passed
@AmelBawa-msft
AmirMS (AmelBawa-msft) deleted the user/amelbawa/fix-zip-symlink-bounds branch November 12, 2026 04:59
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.

5 participants