search

pkg / sftp

SFTP support for the go.crypto/ssh package
BSD 2-Clause "Simplified" License
1.52k stars 380 forks source link

client fix potential crash if we receive a short packet #411

Closed drakkan closed 3 years ago

drakkan commented 3 years ago

This was discovered using continuous fuzzing, see here

greatroar commented 3 years ago

There's actually a lot more of these unsafe unmarshalUint32 calls, in ReadDir, openDir, Stat, Lstat, ReadLink, open, RealPath, readChunkAt, writeChunkAt, unmarshalStatus. The fuzzer won't find them, because it connects and immediately disconnects, without doing any actual work.

Rather than fix all of these by doing unmarshalUint32Safe, maybe recvPacket should take a minSize argument and return errShortPacket if it gets less than minSize bytes?

puellanivis commented 3 years ago

There's actually a lot more of these unsafe unmarshalUint32 calls, in ReadDir, openDir, Stat, Lstat, ReadLink, open, RealPath, readChunkAt, writeChunkAt, unmarshalStatus. The fuzzer won't find them, because it connects and immediately disconnects, without doing any actual work.

In this case, let’s not worry too much about it. I don’t think we could easily push a minSize argument into the recvPacket. And properly addressing this issue might benefit enormously from stepping back, looking at what we’re doing, and using a new paradigm.