Added safety checks for region file validity#393
Merged
Conversation
Check size, check header size, check location table offsets point to valid locations, check for shared offsets, prevent issues with corrupted or junk data
JackNoordhuis
approved these changes
Mar 28, 2017
dktapps
added a commit
that referenced
this pull request
Apr 27, 2017
Check size, check header size, check location table offsets point to valid locations, check for shared offsets, prevent issues with corrupted or junk data
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
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.
Recently it was brought to my attention, by someone having freeze issues on their server, that corrupted or invalid region files can really mess up your server.
In the process of testing their issue I discovered that the RegionLoader does not impose any checks whatsoever on region file validity. You can feed any random crap into there and PocketMine will attempt to use it.
My test:
The reason for said freeze was due to an invalid location table. Region files begin with an 8192-byte header, the first 4096 bytes of which are chunk locations. The first 3 bytes of each entry is the file offset in 4KiB sectors, and the last byte is the length in sectors. Trying to read a garbage file like the one above produces some extremely large offsets for chunks that don't actually exist, so when the server writes to that offset, it also writes billions of zeros in between.
This is a very critical issue since the process cannot be killed while it is writing to disk. In the process of debugging this issue I had to force-kill my laptop half a dozen times.
This pull request adds tight restrictions on region file validity as per the specification here to prevent the issue described above occurring.
If any of the detailed checks fail, the region file will be considered corrupted, will be renamed with a backup name, and a new one will be generated. You may then attempt to use MCEdit or other tools to attempt to repair the defective region.