Skip to content

CFE-4687: Fixed off-by-one in protocol recv buffers#6177

Open
nickanderson wants to merge 2 commits into
cfengine:masterfrom
nickanderson:CFE-4687/protocol-recv-overflow-tests
Open

CFE-4687: Fixed off-by-one in protocol recv buffers#6177
nickanderson wants to merge 2 commits into
cfengine:masterfrom
nickanderson:CFE-4687/protocol-recv-overflow-tests

Conversation

@nickanderson

Copy link
Copy Markdown
Member

ProtocolOpenDir()/ProtocolGet() receive up to CF_MSGSIZE bytes into a char buf[CF_MSGSIZE], but the receive primitives NUL-terminate at buf[received] where received can equal CF_MSGSIZE -- one past the array (TLSRecv/RecvSocketStream both write toget+1 bytes).

Drives the real ProtocolOpenDir() over a classic-protocol socketpair with a record-filling reply. Aborts under AddressSanitizer on current code, passes once the buffers are sized CF_BUFSIZE (#6171).

Ref: CFE-4687, #6171

nickanderson and others added 2 commits June 12, 2026 11:38
ProtocolOpenDir()/ProtocolGet() receive up to CF_MSGSIZE bytes into a
char buf[CF_MSGSIZE], but the receive primitives NUL-terminate at
buf[received] where received can equal CF_MSGSIZE -- one past the array
(TLSRecv/RecvSocketStream both write toget+1 bytes).

Drives the real ProtocolOpenDir() over a classic-protocol socketpair with
a record-filling reply. Aborts under AddressSanitizer on current code,
passes once the buffers are sized CF_BUFSIZE (cfengine#6171).

Ref: CFE-4687, cfengine#6171

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@nickanderson nickanderson changed the title Added test to surface off-by-one in protocol recv buffers (CFE-4687) CFE-4687: Fixed off-by-one in protocol recv buffers Jun 12, 2026
@nickanderson

Copy link
Copy Markdown
Member Author

@cf-bottom jenkins please

@cf-bottom

Copy link
Copy Markdown

@olehermanse olehermanse left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

At first glance, I'd say it sounds more appropriate that the buffers should be +1 (CF_MSGSIZE + 1) to account for the NUL-byte. However, after reading the comment in your test, I see that CF_BUFSIZE is already used in another place for the same thing (STAT), so it makes some sense to do the same as there.

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

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants