-
Notifications
You must be signed in to change notification settings - Fork 63
Adds adjustable ZipSecureFile Inflation Ratio for AAS Environment #938
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This pull request adds a configurable minimum inflate ratio setting for ZIP file processing in the AAS Environment HTTP controller to help mitigate ZIP bomb vulnerabilities. The change allows administrators to control how much compressed ZIP data can expand in memory during decompression.
Key Changes:
- Added
basyx.aasenvironment.minInflateRatioconfiguration property with default value of 1.0 - Integrated Apache POI's
ZipSecureFile.setMinInflateRatio()to apply the configured security threshold - Applied the setting in the
AasEnvironmentApiHTTPControllerconstructor
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| basyx.aasenvironment/basyx.aasenvironment.component/src/main/resources/application.properties | Added commented configuration property for minimum inflate ratio with example value |
| basyx.aasenvironment/basyx.aasenvironment-http/src/main/java/org/eclipse/digitaltwin/basyx/aasenvironment/http/AasEnvironmentApiHTTPController.java | Added ZipSecureFile import, @Value-annotated field for the configuration property, and initialization logic in the constructor |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| private static final String ACCEPT_AASX = "application/asset-administration-shell-package+xml"; | ||
|
|
||
| @Value("${basyx.aasenvironment.minInflateRatio:1.0}") | ||
| public double minInflateRatio; |
Copilot
AI
Dec 11, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The field visibility should be private rather than public. This field is injected via Spring's @value annotation and should not be accessible or modifiable from outside the class. Making it public exposes internal implementation details and could allow unintended modification.
| public double minInflateRatio; | |
| private double minInflateRatio; |
| public AasEnvironmentApiHTTPController(HttpServletRequest request, AasEnvironment aasEnvironment) { | ||
| this.request = request; | ||
| this.aasEnvironment = aasEnvironment; | ||
| ZipSecureFile.setMinInflateRatio(minInflateRatio); |
Copilot
AI
Dec 11, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The minInflateRatio field is used before Spring has a chance to inject the value from the @value annotation. When a @RestController is instantiated, Spring first calls the constructor, and only after that performs field injection. At the time line 76 executes, minInflateRatio will still be 0.0 (the default value for double primitives), not the configured value or the default of 1.0. This means ZipSecureFile will always be configured with 0.0, which is likely not the intended behavior and could cause security issues or runtime failures.
| public AasEnvironmentApiHTTPController(HttpServletRequest request, AasEnvironment aasEnvironment) { | ||
| this.request = request; | ||
| this.aasEnvironment = aasEnvironment; | ||
| ZipSecureFile.setMinInflateRatio(minInflateRatio); |
Copilot
AI
Dec 11, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ZipSecureFile.setMinInflateRatio() is a static method that modifies a global setting. This creates multiple issues: (1) In a multi-threaded environment, each controller instance will overwrite the global setting, potentially causing race conditions. (2) If multiple controllers are instantiated with different configurations (e.g., in integration tests), they will interfere with each other. (3) The setting persists across all ZIP file operations in the entire JVM, not just for this controller. Consider setting this value once during application initialization in a configuration class annotated with @PostConstruct or in a Spring @configuration class, rather than in each controller instance constructor.
| # basyx.cors.allowed-origins=http://localhost:3000, http://localhost:4000 | ||
| # basyx.cors.allowed-methods=GET,POST,PATCH,DELETE,PUT,OPTIONS,HEAD | ||
|
|
||
| # basyx.aasenvironment.minInflateRatio=0.00001 |
Copilot
AI
Dec 11, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The commented example value of 0.00001 is extremely permissive and could allow ZIP bombs to expand 100,000 times (1/0.00001). This value would significantly weaken security protections. The comment should either: (1) Include documentation explaining what this ratio means (e.g., "Minimum ratio of compressed to uncompressed size; lower values allow more expansion"), (2) Show a more secure example value, or (3) Explain when and why someone would need to lower this value from the default of 1.0.
| # basyx.aasenvironment.minInflateRatio=0.00001 | |
| # basyx.aasenvironment.minInflateRatio=1.0 | |
| # Minimum ratio of compressed to uncompressed size for uploaded ZIP files. | |
| # Lower values allow more expansion and may increase risk of ZIP bomb attacks. | |
| # Recommended: keep at 1.0 for maximum security. Only lower if you have a specific need and understand the risks. |
- Adds Error Message
Description of Changes
This pull request introduces a configurable security improvement to the AAS Environment HTTP controller by allowing the minimum inflate ratio for ZIP file processing to be set via a property. This helps mitigate ZIP bomb vulnerabilities by controlling how much compressed data can expand in memory. The most important changes are:
Security and Configuration:
basyx.aasenvironment.minInflateRatio, to set the minimum inflate ratio for ZIP file decompression, with a default value of0.01. This property can be set inapplication.propertiesto customize the behavior. (F4845655L62R62, basyx.aasenvironment/basyx.aasenvironment.component/src/main/resources/application.propertiesR23-R24)minInflateRatioproperty into theAasEnvironmentApiHTTPControllerusing Spring's@Valueannotation. [1] [2]ZipSecureFilein the controller's constructor, ensuring the value is applied at runtime. [1] [2]Related Issue
BaSyx Configuration for Testing
AAS Files Used for Testing
Additional Information