Skip to content
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

Fix crashes/freezes in VSS stripping code #913

Merged
merged 2 commits into from
Apr 14, 2023
Merged

Fix crashes/freezes in VSS stripping code #913

merged 2 commits into from
Apr 14, 2023

Conversation

raineth
Copy link
Contributor

@raineth raineth commented Apr 5, 2023

Greetings,

I ran into a couple of issues while attempting to restore a large backup from a Windows host with VSS stripping enabled:

  • When VSS headers happen to span across 2 calls to bfile_write_vss_strip(), memory corruption can occur. This manifested as a segfault for me.
  • Removing VSS headers from a file containing an empty stream results in an infinite loop. This can happen in a sparse file that has 0s at the beginning; this is the output of vss_strip -p for a file that caused the issue for me.
VSS header: 1 8 0 0
VSS header: 9 8 5701640 0
VSS header: 9 8 1638408 0
VSS header: 9 8 7208968 0
VSS header: 9 8 8 0

There are some more details in the commit messages.

As far as I can tell, bfile_write_vss_strip() and the vss_strip tool do not properly support sparse files and will only write sections with type 1 to disk, but with these fixes I can at least restore my backup and address the sparse files some other way if needed. I couldn't easily find the information I'd need to implement -x for BACKUP_SPARSE_BLOCK (type 9), but I'm sure I'm preaching to the choir there.

I discovered issue #895 while researching my problems, and I suspect that issue was caused either by the first bug or the lack of sparse file support.

Cheers!

When VSS header data straddles multiple network blocks (i.e. got <
bsidsize in bfile_write_vss_strip()), the pointer arithmatic when
memcpy()ing the next block is incorrect.

Since sid is a pointer to a structure, each addition by 1 increases
the pointer address by sizeof(*sid); therefore, when sidlen > 0, the
memcpy() unintentionally overwrites part of the struct BFILE it lives
within or the memory situated after it.

Casting sid to a char* in the memcpy() causes the addition to advance
the pointer by single bytes as intended.
When stripping VSS data, an empty stream that has additional data
following it will currently cause an infinite loop in
bfile_write_vss_strip().

When encountering an empty stream, attempt to start reading a new VSS
header instead.
@grke
Copy link
Owner

grke commented Apr 7, 2023

Hello, thanks for this, I will check it soon.

@grke grke merged commit ed1e60f into grke:master Apr 14, 2023
@grke
Copy link
Owner

grke commented Apr 14, 2023

This patch doesn't break any of my automated tests, and looks fine to me, so happily merged.
Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants