fix: handle extra field padding

[Its-Just-Nans]

fix: #681

Need to check if we handle it even if it looks like not from the specs

[gemini-code-assist]

Summary of Changes

Hello @Its-Just-Nans, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request addresses an issue where the ZIP parser could incorrectly report errors when encountering malformed or truncated extra fields, particularly those that might be padding. The changes introduce more sophisticated logic to distinguish between known extra fields and unknown ones, allowing the parser to gracefully skip over truncated unknown fields while providing more informative errors for known ones. This improves the robustness of the ZIP reader when dealing with non-standard or partially written extra field data.

Highlights

• Improved Extra Field Parsing: The parsing logic for extra fields has been enhanced to differentiate between known and unknown extra field types when encountering truncated data. This allows for more robust handling of potentially malformed or padded extra fields.
• Graceful Padding Handling: Unknown extra fields that are truncated (i.e., their length cannot be fully read) are now gracefully handled as potential padding, preventing unnecessary errors and allowing parsing to continue.
• Enhanced Error Messages: Error messages for truncated known extra fields now include the specific field type, providing more context for debugging.
• Code Structure Refinement: Module imports and re-exports in src/extra_fields/mod.rs were reordered for better code organization, and the Display trait was implemented for UsedExtraField.

Changelog

• src/extra_fields/mod.rs
  • Moved module imports and re-exports to the top of the file.
  • Implemented the Display trait for UsedExtraField to enable hexadecimal formatting in error messages.
• src/read.rs
  • Modified parse_single_extra_field to first attempt to decode the extra field kind before reading its length.
  • Introduced an early return (Ok(false)) for unknown extra fields where the length cannot be fully read, treating them as potential padding.
  • Updated error messages for truncated known extra fields to include the specific field identifier.

Activity

• No specific activity (comments, reviews, etc.) has been recorded for this pull request yet.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[amazon-q-developer]

The changes improve extra field parsing by handling padding more gracefully while maintaining strict validation for known fields. The implementation correctly differentiates between known and unknown extra fields, treating truncated unknown fields as padding (per the ZIP specification's allowance for padding) while still enforcing proper structure for known fields. The refactoring in src/extra_fields/mod.rs improves code organization. No defects found that would block merge.

---

You can now have the agent implement changes and create commits directly on your pull request's source branch. Simply comment with /q followed by your request in natural language to ask the agent to make changes.

[gemini-code-assist]

Code Review

This pull request aims to improve handling of extra field padding by being more lenient when reading unknown extra field lengths, preventing archive read failures due to trailing garbage. However, the proposed changes introduce a critical infinite loop vulnerability in src/read.rs when processing malformed ZIP files with truncated extra field headers, potentially leading to a Denial of Service (DoS) by causing 100% CPU usage. While addressing the padding issue, it's crucial to ensure the parser always makes progress or terminates gracefully with truncated data. Additionally, there are minor improvements suggested for better error reporting and more robust error handling.

[Its-Just-Nans]

@Pr0methean

Could you cancel the merge
Or at least not create a release after the merge?

I think I can improve it a little bit by removing the mode duplication (right now it's inside EncryptedWith and CustomSalt).

[Pr0methean]

Canceled.

[Its-Just-Nans]

Canceled.

Thanks
Sorry the previous comment about CustomSalt - it was about the MR #680

I will add test(s) on this pull request soon

[Pr0methean]

Looks good; just a few nitpicks about the test.