Skip to content

fix off-by-one in ProtocolGet and ProtocolOpenDir recv buffers#6171

Open
aizu-m wants to merge 1 commit into
cfengine:masterfrom
aizu-m:protocol-recv-buf-off-by-one
Open

fix off-by-one in ProtocolGet and ProtocolOpenDir recv buffers#6171
aizu-m wants to merge 1 commit into
cfengine:masterfrom
aizu-m:protocol-recv-buf-off-by-one

Conversation

@aizu-m

@aizu-m aizu-m commented Jun 9, 2026

Copy link
Copy Markdown

Tracing the file-receive path in libcfnet/protocol.c.

TLSRecv() NUL-terminates what it reads:

tls_generic.c:  buffer[received] = '\0';   // received can equal toget

So the buffer must hold toget + 1 bytes. net.c does exactly that already
with char proto[CF_INBAND_OFFSET + 1] for a CF_INBAND_OFFSET read.

ProtocolGet() and ProtocolOpenDir() declare buf[CF_MSGSIZE] yet receive
up to CF_MSGSIZE bytes. GET goes straight through TLSRecv(ssl, buf, CF_MSGSIZE). OPENDIR goes through ReceiveTransaction(), whose length cap is
CF_BUFSIZE - CF_INBAND_OFFSET, i.e. CF_MSGSIZE. A peer that fills the
record makes received reach CF_MSGSIZE, so the terminating NUL is written
to buf[CF_MSGSIZE], one byte past the array. The byte count is server
controlled, so the reach crosses the trust boundary.

Confirmed by modelling the terminating write with a guard byte placed right
after the buffer: the guard is clobbered at CF_MSGSIZE, intact once the
buffer is CF_BUFSIZE.

ProtocolStat() in the same file already sizes its buffer CF_BUFSIZE. The
other two now match it.

@cf-bottom

Copy link
Copy Markdown

Thank you for submitting a pull request! Maybe @craigcomstock can review this?

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.

2 participants