vadp-dumper: fix out of bounds read#1908
Merged
BareosBot merged 8 commits intobareos:masterfrom Aug 9, 2024
Merged
Conversation
8d7efe4 to
ba46534
Compare
sduehr
requested changes
Aug 3, 2024
Member
sduehr
left a comment
There was a problem hiding this comment.
After I understood the intersection algorithm, the requested change seems obvious to me.
7fdaad9 to
f42dd7a
Compare
sduehr
requested changes
Aug 5, 2024
Member
sduehr
left a comment
There was a problem hiding this comment.
Didn't notice it before, subtracting offset from std::min(bend, oend) is missing to get min_length.
9d01fc4 to
b32a30d
Compare
sduehr
approved these changes
Aug 6, 2024
Member
sduehr
left a comment
There was a problem hiding this comment.
Thanks a lot, especially for the extra checks and variable renaming, which make the code a log better readable.
6 tasks
The read_handle should always get closed, not just when we try to process the cbts.
The names were completely out of sync for things that were basically the same. This made it very hard to understand the algorithm as well as generally being hard to see logic errors.
The early exit means that we do not correctly compute saved_len which in turn causes us to misrepresent how much data was actually saved with that api.
609a4a0 to
e50248e
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Thank you for contributing to the Bareos Project!
We previously did not properly take into account the possibility that there could be unallocated, but changed blocks.
Specifically at the end of a volume. This caused us to read unallocated memory.
This PR adds bounds check to our vector implementation as well as properly handling that case during the processing.
Please check
If you have any questions or problems, please give a comment in the PR.
Helpful documentation and best practices
Checklist for the reviewer of the PR (will be processed by the Bareos team)
Make sure you check/merge the PR using
devtools/pr-toolto have some simple automated checks run and a proper changelog record added.General
Source code quality