diff --git a/ptp/protocol/tlvs.go b/ptp/protocol/tlvs.go index 3d598609..972ca923 100644 --- a/ptp/protocol/tlvs.go +++ b/ptp/protocol/tlvs.go @@ -378,17 +378,16 @@ func (t *AlternateTimeOffsetIndicatorTLV) MarshalBinaryTo(b []byte) (int, error) binary.BigEndian.PutUint32(b[tlvHeadSize+5:], uint32(t.JumpSeconds)) copy(b[tlvHeadSize+9:], t.TimeOfNextJump[:]) //uint48 size := tlvHeadSize + 15 - if t.DisplayName != "" { - dd, err := t.DisplayName.MarshalBinary() - if err != nil { - return 0, fmt.Errorf("writing AlternateTimeOffsetIndicatorTLV DisplayName: %w", err) - } - if len(b) < size+len(dd) { - return 0, fmt.Errorf("not enough buffer to marshal AlternateTimeOffsetIndicatorTLV with DisplayName, need %d, have %d", size+len(dd), len(b)) - } - copy(b[tlvHeadSize+15:], dd) - size += len(dd) + // DisplayName is always present on the wire, an empty one is a single zero length octet + dd, err := t.DisplayName.MarshalBinary() + if err != nil { + return 0, fmt.Errorf("writing AlternateTimeOffsetIndicatorTLV DisplayName: %w", err) + } + if len(b) < size+len(dd) { + return 0, fmt.Errorf("not enough buffer to marshal AlternateTimeOffsetIndicatorTLV with DisplayName, need %d, have %d", size+len(dd), len(b)) } + copy(b[tlvHeadSize+15:], dd) + size += len(dd) return size, nil } @@ -397,14 +396,15 @@ func (t *AlternateTimeOffsetIndicatorTLV) UnmarshalBinary(b []byte) error { if err := unmarshalTLVHeader(&t.TLVHead, b); err != nil { return err } - if err := checkTLVLength(&t.TLVHead, len(b), 20, false); err != nil { + // 15 octets of fixed fields followed by DisplayName, which is at least its length octet + if err := checkTLVLength(&t.TLVHead, len(b), 16, false); err != nil { return err } t.KeyField = b[tlvHeadSize] t.CurrentOffset = int32(binary.BigEndian.Uint32(b[tlvHeadSize+1:])) t.JumpSeconds = int32(binary.BigEndian.Uint32(b[tlvHeadSize+5:])) copy(t.TimeOfNextJump[:], b[tlvHeadSize+9:]) // uint48 - if err := t.DisplayName.UnmarshalBinary(b[tlvHeadSize+15:]); err != nil { + if err := t.DisplayName.UnmarshalBinary(b[tlvHeadSize+15 : tlvHeadSize+int(t.LengthField)]); err != nil { return fmt.Errorf("reading AlternateTimeOffsetIndicatorTLV DisplayName: %w", err) } return nil diff --git a/ptp/protocol/tlvs_test.go b/ptp/protocol/tlvs_test.go index 390566a8..8b7cd585 100644 --- a/ptp/protocol/tlvs_test.go +++ b/ptp/protocol/tlvs_test.go @@ -213,6 +213,53 @@ func TestParseAnnounceWithAlternateTimeOffsetIndicator(t *testing.T) { require.Equal(t, &want, pp) } +func TestParseAnnounceWithAlternateTimeOffsetIndicatorEmptyDisplayName(t *testing.T) { + // same Announce as above, but the ALTERNATE_TIME_OFFSET_INDICATOR TLV carries an empty displayName, + // so its lengthField is 16: 15 octets of fixed fields plus the zero displayName length octet + raw := []uint8("\x0b\x12\x00\x54\x00\x00\x04\x08\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x08\xc0\xeb\xff\xfe\x63\x7a\x4e\x00\x01\x00\x00\x05\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x25\x00\x80\xf8\xfe\xff\xff\x80\x08\xc0\xeb\xff\xfe\x63\x7a\x4e\x00\x00\xa0\x00\x09\x00\x10\x01\x00\x00\x00\x25\x00\x00\x00\x01\x00\x00\x62\xc2\xfd\xb6\x00\x00\x00") + packet := new(Announce) + err := FromBytes(raw, packet) + require.NoError(t, err) + want := []TLV{ + &AlternateTimeOffsetIndicatorTLV{ + TLVHead: TLVHead{ + TLVType: TLVAlternateTimeOffsetIndicator, + LengthField: 16, + }, + KeyField: 0x01, + CurrentOffset: 37, + JumpSeconds: 1, + TimeOfNextJump: NewPTPSeconds(time.Unix(1656946102, 0)), + }, + } + require.Equal(t, want, packet.TLVs) + b, err := Bytes(packet) + require.NoError(t, err) + require.Equal(t, raw, b) +} + +func TestParseAlternateTimeOffsetIndicatorDisplayNameOverrun(t *testing.T) { + // lengthField is 20, but the displayName length octet claims 8 octets, + // which would run into the ALTERNATE_RESPONSE_PORT TLV that follows + raw := []uint8("\x00\x09\x00\x14\x01\x00\x00\x00\x25\x00\x00\x00\x01\x00\x00\x62\xc2\xfd\xb6\x08PTP\x00\x80\x01\x00\x02\x00\x05") + tlv := &AlternateTimeOffsetIndicatorTLV{} + err := tlv.UnmarshalBinary(raw) + require.ErrorContains(t, err, "DisplayName") +} + +func TestAlternateTimeOffsetIndicatorOneCharDisplayName(t *testing.T) { + // 15 octets of fixed fields, the displayName length octet, one character and one pad octet + raw := []uint8("\x00\x09\x00\x12\x01\x00\x00\x00\x25\x00\x00\x00\x01\x00\x00\x62\xc2\xfd\xb6\x01X\x00") + tlv := &AlternateTimeOffsetIndicatorTLV{} + err := tlv.UnmarshalBinary(raw) + require.NoError(t, err) + require.Equal(t, PTPText("X"), tlv.DisplayName) + b := make([]byte, len(raw)) + n, err := tlv.MarshalBinaryTo(b) + require.NoError(t, err) + require.Equal(t, raw, b[:n]) +} + func TestParseSyncDelayReqWithAlternateResponsePort(t *testing.T) { raw := []byte{1, 18, 0, 50, 0, 0, 36, 0, 0, 0, 0, 0, 6, 32, 0, 2, 0, 0, 0, 0, 184, 206, 246, 255, 254, 68, 148, 144, 0, 1, 149, 17, 0, 127, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 32, 7, 0, 2, 16, 146, 0, 0} packet := new(SyncDelayReq)