BurntSushi / rust-snappy

Snappy compression implemented in Rust (including the Snappy frame format).
BSD 3-Clause "New" or "Revised" License
444 stars 43 forks source link

Fix out of bounds write #14

Closed rikardfalkeborn closed 6 years ago

rikardfalkeborn commented 7 years ago

This fixes issue #12.

In read_copy(), when copying data when the pointers still may overlap, the worst case scenario is when the diff is 3. To see this, the following copies will be made:

[-3, 12] -> [0, 15]
[-3, 12] -> [3, 18]
[-3, 12] -> [9, 24]

That is, the copy may write 24 bytes past the current position. Adjust the check to only enter the if-statement if there are 24 or more bytes available. Also, update comments to reflect this, add a debug_assert which verifies that no writes outside the buffer occur.

Two tests are added. One test that illustrates the problem before this change, and one test case that triggers the code path after this change.

This may in some cases cause more slow copies byte by byte. No noticable slowdown was noticed when running bench with and without this change.

For reference, the size of how much past the end the slow overlaps may write for different pointer diffs are shown below. Only diffs up to 15 are show, since if the diff is 16, this code will not be run. Note that before this change, the condition was conservative in the sense that when the diff was 1, the worst case is actually 22, and not 23.

Pointer diff: 1
   [-1, 14] -> [0, 15]
   [-1, 14] -> [1, 16]
   [-1, 14] -> [3, 18]
   [-1, 14] -> [7, 22]

Pointer diff: 2
   [-2, 13] -> [0, 15]
   [-2, 13] -> [2, 17]
   [-2, 13] -> [6, 21]

Pointer diff: 3
   [-3, 12] -> [0, 15]
   [-3, 12] -> [3, 18]
   [-3, 12] -> [9, 24]

Pointer diff: 4
   [-4, 11] -> [0, 15]
   [-4, 11] -> [4, 19]

Pointer diff: 5
   [-5, 10] -> [0, 15]
   [-5, 10] -> [5, 20]

Pointer diff: 6
   [-6, 9] -> [0, 15]
   [-6, 9] -> [6, 21]

Pointer diff: 7
   [-7, 8] -> [0, 15]
   [-7, 8] -> [7, 22]

Pointer diff: 8
   [-8, 7] -> [0, 15]

Pointer diff: 9
   [-9, 6] -> [0, 15]

Pointer diff: 10
   [-10, 5] -> [0, 15]

Pointer diff: 11
   [-11, 4] -> [0, 15]

Pointer diff: 12
   [-12, 3] -> [0, 15]

Pointer diff: 13
   [-13, 2] -> [0, 15]

Pointer diff: 14
   [-14, 1] -> [0, 15]

Pointer diff: 15
   [-15, 0] -> [0, 15]
rikardfalkeborn commented 6 years ago

Any comment on this?

BurntSushi commented 6 years ago

@rikardfalkeborn This PR is in snap 0.2.3 on crates.io. Thanks again!