Skip to content
This repository has been archived by the owner on Feb 18, 2024. It is now read-only.

Panic-free read of IPC files #1075

Merged
merged 2 commits into from
Jun 18, 2022
Merged

Panic-free read of IPC files #1075

merged 2 commits into from
Jun 18, 2022

Conversation

jorgecarleitao
Copy link
Owner

@jorgecarleitao jorgecarleitao commented Jun 15, 2022

This PR safeguards the IPC interface from panicking or aborting (OOM) on invalid data.

  • All number casts (e.g. i32 to usize) and operations (e.g. *) are now protected to avoid overflow and thus trigger OOM
  • Many of the errors were converted to an enum for more clarity over what failed exactly
  • the sum of size of all IPC buffers can no longer be larger than the size of the file, thus protecting us from OOM attacks

@jorgecarleitao jorgecarleitao added the bug Something isn't working label Jun 15, 2022
@codecov
Copy link

codecov bot commented Jun 16, 2022

Codecov Report

Merging #1075 (34b59c3) into main (76d2a39) will increase coverage by 0.27%.
The diff coverage is 86.42%.

@@            Coverage Diff             @@
##             main    #1075      +/-   ##
==========================================
+ Coverage   81.01%   81.29%   +0.27%     
==========================================
  Files         365      367       +2     
  Lines       35023    35525     +502     
==========================================
+ Hits        28373    28879     +506     
+ Misses       6650     6646       -4     
Impacted Files Coverage Δ
src/io/flight/mod.rs 0.00% <0.00%> (ø)
src/io/ipc/mod.rs 100.00% <ø> (+60.00%) ⬆️
src/io/ipc/read/file_async.rs 60.68% <45.09%> (-0.67%) ⬇️
src/io/ipc/read/stream_async.rs 77.34% <77.77%> (+2.96%) ⬆️
src/array/fixed_size_binary/mod.rs 87.37% <80.00%> (-0.26%) ⬇️
src/array/fixed_size_list/mod.rs 63.92% <80.00%> (+0.28%) ⬆️
src/io/ipc/read/common.rs 95.06% <87.23%> (+0.61%) ⬆️
src/io/ipc/read/reader.rs 96.18% <94.93%> (+7.10%) ⬆️
src/io/ipc/read/stream.rs 91.24% <96.29%> (+4.31%) ⬆️
src/io/ipc/read/array/binary.rs 91.93% <100.00%> (+0.55%) ⬆️
... and 29 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 76d2a39...34b59c3. Read the comment docs.

@jorgecarleitao jorgecarleitao marked this pull request as ready for review June 16, 2022 17:01
@jorgecarleitao jorgecarleitao changed the title panic free read of uncompressed IPC files Panic-free read of IPC files Jun 16, 2022
@jorgecarleitao jorgecarleitao force-pushed the ipc_no_panic branch 4 times, most recently from 422423d to 5971d89 Compare June 16, 2022 17:42
@jorgecarleitao
Copy link
Owner Author

Thanks @kristoff3r and @TethysSvensson for the quick fix and release of planus-org/planus#99 !

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant