Deuxfleurs/garage
45
52
Fork 67
Code Issues 92 Pull requests 9 Releases 29 Wiki Activity

Support STREAMING-AWS4-HMAC-SHA256-PAYLOAD (#64) #156

Merged
lx merged 11 commits from KokaKiwi/garage:aws4-payload-signing into main 2022-01-17 09:55:31 +00:00
Conversation 0 Commits 11 Files changed 13 -4
1 changed files with 0 additions and 4 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files
Showing only changes of commit b2eda2c13e - Show all commits

4
src/api/signature/streaming.rs
View file

@ -233,10 +233,6 @@ where
return Poll::Ready(Some(Err(SignedPayloadStreamError::Stream(e)))) return Poll::Ready(Some(Err(SignedPayloadStreamError::Stream(e))))
} }
None => { None => {
if this.buf.is_empty() {
return Poll::Ready(None);
}
return Poll::Ready(Some(Err(SignedPayloadStreamError::message( return Poll::Ready(Some(Err(SignedPayloadStreamError::message(
"Unexpected EOF", "Unexpected EOF",
)))); ))));
KokaKiwi marked this conversation as resolved Outdated
trinity-1686a commented 2022-01-12 18:28:18 +00:00
Outdated
Review
Copy link

this bit has an invalid edge case : by cutting the stream just before a new chunk header, an attacker can truncate the file without it being rejected. Getting here (inner stream returns None and this.buf is empy) is either such a truncation, or a call to SignedPayloadStream::poll_next after it returned Ok(Ready(None)) once, which is a contract error (Ok(Ready(None)) means that the stream has terminated, and poll_next should not be invoked again), so this check and return can be safely removed

this bit has an invalid edge case : by cutting the stream just before a new chunk header, an attacker can truncate the file without it being rejected. Getting here (inner stream returns None and this.buf is empy) is either such a truncation, or a call to SignedPayloadStream::poll_next after it returned Ok(Ready(None)) once, which is a contract error ([`Ok(Ready(None)) means that the stream has terminated, and poll_next should not be invoked again`](https://docs.rs/futures/0.2.0/futures/stream/trait.Stream.html#return-value)), so this check and return can be safely removed