Skip to content

Fix/tlv unknown arbitrary offset#86

Merged
brainstorm merged 2 commits into
brainstorm:mainfrom
jubeormk1:fix/tlv-unknown-arbitrary-offset
Apr 1, 2026
Merged

Fix/tlv unknown arbitrary offset#86
brainstorm merged 2 commits into
brainstorm:mainfrom
jubeormk1:fix/tlv-unknown-arbitrary-offset

Conversation

@jubeormk1

Copy link
Copy Markdown
Collaborator

This PR is focus on Radically Open Security report on the possible arbitrary offset injection on TLV parameters that may lead to undefined behaviour.

An unknown TLV element in the ota packet now is rejected by the ota handler. The SftpServer response to the Write request in this case is sending back to the client a SSH_FX_OP_UNSUPPORTED status.

This fix blocks the client but does not block the server, that can receive other ssh or sftp connections after the failure.

TLV unknown packet can arbitrarily advance the stream. The decision here is disabling skipping unknown TLV types and instead let the ota handler fail
The result of this change is that on sending an incorrect (crafted) ota file the ssh-server aborts the ota-update. The client receives a Failure but that is not enough to close the connection.

The crafted file was created editing the type immediately after OtaType (value 0) and giving it a value out of range (0x09)
@jubeormk1 jubeormk1 self-assigned this Apr 1, 2026
@brainstorm brainstorm self-requested a review April 1, 2026 19:55

@brainstorm brainstorm left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if that error handling belongs on the side of SSH Stamp, but I'll be approving it now just in case I'm flat wrong: you decide, @jubeormk1 ;)

EDIT: Merging it after disabling the hil-ready branch rules, we'll re-enable them later... open another PR addressing my comments (if needed at all!).

@brainstorm brainstorm merged commit 3ed66bb into brainstorm:main Apr 1, 2026
8 checks passed
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