darrenldl / blockyarchive

Blocky archive - multithreaded archiver offering bit rot protection and sector level recoverability
MIT License
94 stars 4 forks source link

Panic in repair mode when corruption changes block version number to different one #191

Closed darrenldl closed 5 years ago

darrenldl commented 5 years ago

The panic originates from check_buffer! at sbx_block::mod.rs:812, in the function sync_from_buffer in corruption_tests_encode_stdin.sh test, when the header version number is corrupted to become another version number. Blkar was in repair mode when the panic happened.

When the block size associated with the header's version number is bigger than the provided buffer in sync_from_buffer, it panics via check_buffer!. This behaviour, however, means it neglects completely the possibility of header being malformed and when we are anticipating with a specific block size. However, it is impossible to tell with certainty whether the header is well formed or not due to SBX format design, so blkar is forced to trust the header at various places as well.

So the best remedy is to return error along the line of InsufficientBufferSize for sync_from_buffer and maybe other similar functions.

darrenldl commented 5 years ago

Actually this is somewhat mitigated already in repair_core by the supplying of pred, but the buffer checking occurs before pred checking takes place.

Right now the pred signature takes a block as input, but most predicates should only require working at header level, so adding a header_pred might be an even better solution, the checking of which takes place before syncing from buffer.

darrenldl commented 5 years ago

Fixed in #192 by moving to using equivalent header predicates.