Add confirmation to large hexdump reads.
Your checklist for this pull request
- [x] I've read the guidelines for contributing to this repository.
- [x] I made sure to follow the project's coding style.
- [x] I've documented every
RZ_APIfunction and struct this PR changes. - [x] I've added tests that prove my changes are effective (required for changes to
RZ_API). - [x] I've updated the Rizin book with the relevant information (if needed).
Detailed description
If you mistakenly provide a very large length parameter to px (in my case, px rsp), rizin will continue the read until it is complete. This can take several minutes, and can't be cancelled with SIGINT.
This PR adds a check via rz_cons_yesno(), requesting the user to confirm hexdumps above some threshhold. For now, this threshhold is 0x1000.
If a rz_core_print_hexdump_or_hexdiff_str() returns NULL, an additional error message is printed. To avoid this message in case the user answers n to the confirmation, the length is now normalised and confirmed before calling rz_core_print_hexdump_or_hexdiff_str().
I imagine the real solution here is to register a temporary SIGINT handler that would cancel the read. I'm inclined to say that an even better solution is probably to register a generic SIGINT handler, and have it call a callback for whichever operation is currently ongoing. I'm not sure how feasible that is given the current architecture. I'm also not familiar with the codebase enough yet to implement something like this.
...
Test plan
$ rizin -d a.out
> s main
> px 0x40000000
[attempt both y & n paths]
I'd be happy to define some tests for this if you'd like.
...
Closing issues
N/A ...
I should note that other calls to rz_core_print_hexdump_or_hexdiff_str() pass constants below core->blocksize_max as the length (core->blocksize & 128), so normalisation can safely be moved up the callstack here.
Formatting [84/1854] librz/core/cprint.c
librz/core/cprint.c:390:18: error: code should be clang-formatted [-Wclang-format-violations]
if (len > 0x1000
^
Codecov Report
Attention: Patch coverage is 25.00000% with 3 lines in your changes missing coverage. Please review.
Project coverage is 44.87%. Comparing base (
76d41f7) to head (0fa999c). Report is 8 commits behind head on dev.
| Files with missing lines | Patch % | Lines |
|---|---|---|
| librz/core/cprint.c | 25.00% | 2 Missing and 1 partial :warning: |
Additional details and impacted files
| Files with missing lines | Coverage Δ | |
|---|---|---|
| librz/core/cprint.c | 65.75% <25.00%> (-0.29%) |
:arrow_down: |
... and 11 files with indirect coverage changes
Continue to review full report in Codecov by Sentry.
Legend - Click here to learn more
Δ = absolute <relative> (impact),ø = not affected,? = missing dataPowered by Codecov. Last update 76d41f7...0fa999c. Read the comment docs.
:rocket: New features to boost your workflow:
- :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
- :package: JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.