validate env limits in qfile-dom0-unpacker#209
validate env limits in qfile-dom0-unpacker#209rishi-jat wants to merge 1 commit intoQubesOS:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR hardens parsing of UPDATES_MAX_BYTES and UPDATES_MAX_FILES in qfile-dom0-unpacker to avoid fail-open behavior when environment variables are malformed, enforcing fail-closed semantics.
Changes:
- Introduces
parse_limit_env()usingstrtoll()with strict validation (full parse, no overflow, no negatives). - Ensures invalid environment values cause an error message and immediate exit.
- Replaces
atoll()-based parsing with the new validated parser for both limits.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #209 +/- ##
=======================================
Coverage 69.33% 69.33%
=======================================
Files 10 10
Lines 1275 1275
=======================================
Hits 884 884
Misses 391 391 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
/cc @ben-grande |
Signed-off-by: Rishi Jat <rishijat098@gmail.com>
rishi-jat
force-pushed
the
fix-validate-env-limits
branch
from
c9e3c08 to
515f84f
Compare
|
@rishi-jat If this was assisted by AI you need to mention that it was. See the Qubes OS documentation. (no longer a team member, but familiar with this rule) |
|
I am no C expert, I just tested the code. Error messages should contain the reason of the failure, to be able to differentiate amongst them. The last if statement seems to be never reached as errors fall under the previous conditions, but it might be ok to keep it. |
OpenQA test summaryComplete test suite and dependencies: https://openqa.qubes-os.org/tests/overview?distri=qubesos&version=4.3&build=2026042817-devel&flavor=pull-requests Test run included the following:
New failures, excluding unstableCompared to: https://openqa.qubes-os.org/tests/overview?distri=qubesos&version=4.3&build=2026032404-devel&flavor=update
Failed tests43 failures
Fixed failuresCompared to: https://openqa.qubes-os.org/tests/170766#dependencies 27 fixed
Unstable testsDetails
Performance TestsPerformance degradation:No issues Remaining performance tests:39 tests
|
Summary
This change fixes unsafe parsing of UPDATES_MAX_BYTES and UPDATES_MAX_FILES in qfile-dom0-unpacker.
The current implementation uses atoll(), which silently returns 0 on invalid input. Since 0 is interpreted as “no limit”, malformed or unintended values (e.g., non-numeric strings) can effectively disable limits without any error. In the context of copying data into dom0, this results in a fail-open behavior and weakens expected safeguards.
This patch replaces atoll() with strtoll() and adds strict validation. The parsing now:
Any invalid input results in an error message and immediate exit, enforcing fail-closed semantics.
This ensures that user misconfiguration cannot silently remove limits and aligns the behavior with the expectation that invalid input must not degrade safety guarantees.
Fixes QubesOS/qubes-issues#8882