Skip to content

Fixed out-of-bounds read in RemoteDirList when parsing a listing - #6390

Open
Dhanu-dynamic wants to merge 1 commit into
cfengine:masterfrom
Dhanu-dynamic:remote-dirlist-packet-bound
Open

Dhanu-dynamic wants to merge 1 commit into
cfengine:masterfrom
Dhanu-dynamic:remote-dirlist-packet-bound

Conversation

@Dhanu-dynamic

Copy link
Copy Markdown

RemoteDirList walks the NUL-separated entries of a server reply with
for (sp = recvbuffer; *sp != '\0'; sp += strlen(sp) + 1), never looking
at the length ReceiveTransaction returned, so a reply that does not end
in the double '\0' carries the scan on into whatever the previous reply
left in the buffer, and off the end of it when that residue holds no NUL.
a hostile policy server can thus synthesise directory entries out of
buffer leftovers, or silently drop the entries of the last packet by
leaving a stale CFD_TERMINATOR where the scan resumes, which is what
the new test does. the encrypted branch discards DecryptString's return
as well, so the plaintext is never terminated and the scan runs past that
too; both lengths are now honoured, the same way ProtocolOpenDir in
libcfnet/protocol.c already bounds this loop. reached as root from any
recursive copy_from, through SourceSearchAndCopy ->
AbstractDirOpen -> RemoteDirList.

`RemoteDirList` walks the NUL-separated entries of a server reply with
`for (sp = recvbuffer; *sp != '\0'; sp += strlen(sp) + 1)`, never looking
at the length `ReceiveTransaction` returned, so a reply that does not end
in the double `'\0'` carries the scan on into whatever the previous reply
left in the buffer, and off the end of it when that residue holds no NUL.
a hostile policy server can thus synthesise directory entries out of
buffer leftovers, or silently drop the entries of the last packet by
leaving a stale `CFD_TERMINATOR` where the scan resumes, which is what
the new test does. the encrypted branch discards `DecryptString`'s return
as well, so the plaintext is never terminated and the scan runs past that
too; both lengths are now honoured, the same way `ProtocolOpenDir` in
`libcfnet/protocol.c` already bounds this loop. reached as root from any
recursive `copy_from`, through `SourceSearchAndCopy` ->
`AbstractDirOpen` -> `RemoteDirList`.

Changelog: Title
Signed-off-by: Dhanraj E <dhanraj@labs.digiscrypt.com>
@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

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