Skip to content

link-prober: Reject truncated command TLVs before dispatch - #353

Open
ashutosh-agrawal wants to merge 1 commit into
sonic-net:masterfrom
ashutosh-agrawal:validate-command-tlv-length
Open

ashutosh-agrawal wants to merge 1 commit into
sonic-net:masterfrom
ashutosh-agrawal:validate-command-tlv-length

Conversation

@ashutosh-agrawal

@ashutosh-agrawal ashutosh-agrawal commented Sep 1, 2026

Copy link
Copy Markdown
Member

Description of PR

Summary:

Check the payload length before processing a command TLV.

A command TLV is expected to contain one Command value. We already verify that the TLV fits within the received packet, but we were not checking that its payload was large enough to hold the command itself. If the length was zero, the handler could read the next byte as the command.

This PR adds the missing length check. Valid command TLVs continue to work as before.

Type of change

  • Bug fix
  • New feature
  • Doc/Design
  • Unit test

Approach

What is the motivation for this PR?

Make sure a command is read only when it is part of the declared TLV payload.

Work item tracking
  • Microsoft ADO (number only): N/A

How did you do it?

Check that the payload is at least sizeof(Command) before passing the TLV to the command handler. Stop processing the TLV sequence when the command payload is too short.

How did you verify/test it?

Added a test with a zero-length command TLV and verified that it does not generate a switch-active event.

LinkProberMockTest.LinkProberActiveStandbyIgnoresShortCommandTlv: passed.

Any platform specific information?

N/A

Documentation

N/A

Signed-off-by: Ashutosh Agrawal <ashu@cisco.com>
@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

@mssonicbld

Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants