Skip to content

Commit c1bc2ff

Browse files
drakkanmjbommar
andcommitted
fix: bound extended-count pre-allocation in unmarshalFileStat
The SSH_FILEXFER_ATTRS decoder allocated the extended-attribute slice directly from the wire-supplied extended_count. This is reachable server-side post-authentication via Request.Attributes() and the SETSTAT/FSETSTAT/OPEN paths, and client-side from a malicious server via Stat/Lstat/Fstat/Open/ReadDir attribute parsing. Bound the pre-allocation to the number of entries that can fit in the remaining bytes (each entry is two length-prefixed strings, i.e. at least 8 bytes) and grow the slice with append. Co-authored-by: Michael Bommarito <michael.bommarito@gmail.com>
1 parent fe6029a commit c1bc2ff

2 files changed

Lines changed: 32 additions & 0 deletions

File tree

packet.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -225,6 +225,12 @@ func unmarshalFileStat(flags uint32, b []byte) (*FileStat, []byte, error) {
225225
return nil, b, err
226226
}
227227

228+
// Each extended attribute occupies at least 8 bytes (two
229+
// length-prefixed strings), so a count larger than len(b)/8 cannot
230+
// fit and is malformed.
231+
if count > uint32(len(b)/8) {
232+
return nil, b, errShortPacket
233+
}
228234
ext := make([]StatExtended, count)
229235
for i := uint32(0); i < count; i++ {
230236
var typ string

packet_test.go

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,32 @@ func TestMarshalUint32(t *testing.T) {
3131
}
3232
}
3333

34+
func TestUnmarshalFileStatExtendedOverflow(t *testing.T) {
35+
// flags = EXTENDED only, extended_count = 0xFFFFFFFF, no entries.
36+
b := marshalUint32(nil, 0xFFFFFFFF)
37+
if _, _, err := unmarshalFileStat(sshFileXferAttrExtended, b); err != errShortPacket {
38+
t.Fatalf("expected errShortPacket, got %v", err)
39+
}
40+
41+
// Well-formed control: a single extended entry must still parse.
42+
b = marshalUint32(nil, 1)
43+
b = marshalString(b, "type")
44+
b = marshalString(b, "data")
45+
fs, _, err := unmarshalFileStat(sshFileXferAttrExtended, b)
46+
if err != nil {
47+
t.Fatalf("unexpected error parsing valid extended attrs: %v", err)
48+
}
49+
if len(fs.Extended) != 1 {
50+
t.Fatalf("got %d extended attrs, want 1", len(fs.Extended))
51+
}
52+
if fs.Extended[0].ExtType != "type" {
53+
t.Errorf("got ext type %q, want %q", fs.Extended[0].ExtType, "type")
54+
}
55+
if fs.Extended[0].ExtData != "data" {
56+
t.Errorf("got ext data %q, want %q", fs.Extended[0].ExtData, "data")
57+
}
58+
}
59+
3460
func TestMarshalUint64(t *testing.T) {
3561
var tests = []struct {
3662
v uint64

0 commit comments

Comments
 (0)