Skip to content

Bound the ALTERNATE_TIME_OFFSET_INDICATOR displayName by the TLV length #590

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
protonnegativo wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Bound the ALTERNATE_TIME_OFFSET_INDICATOR displayName by the TLV length #590

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 12 additions & 12 deletions ptp/protocol/tlvs.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand All @@ -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
Expand Down
47 changes: 47 additions & 0 deletions ptp/protocol/tlvs_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Loading