[BUG] tmux server crashes on malformed custom layout

[Jere Viikari]

Hello,

I found a crash in current tmux master while checking the custom layout
parser with GCC analyzer and ASan/UBSan.

This can be reproduced without any configuration:

tmux -Ltest -f/dev/null new-session -d
tmux -Ltest select-layout '007b,{'

The second command reports that the server exited unexpectedly. 007b is the
correct checksum for the layout payload {, so the input passes the initial
checksum validation but is otherwise malformed.

UBSan reports:

layout-custom.c:391:10: runtime error: member access within null pointer
layout-custom.c:391:10: runtime error: store to null pointer

The relevant call stack is:

#0 layout_construct() at layout-custom.c:391
#1 layout_parse() at layout-custom.c:194
#2 cmd_select_layout_exec() at cmd-select-layout.c:127
#3 cmdq_fire_command() at cmd-queue.c:649

It seems that layout_construct_cell() returns NULL because the payload does not
begin with a valid cell description:

*lc = layout_construct_cell(lcparent, layout);

However, when the next character is { or [, layout_construct() continues and
dereferences *lc:

(*lc)->type = LAYOUT_LEFTRIGHT;

The server should reject this layout as invalid instead of crashing. This seems
to be prevented by checking the result immediately:

*lc = layout_construct_cell(lcparent, layout);
if (*lc == NULL)
goto fail;

Thanks,
Jere

[Nicholas Marriott]

Thanks. It probably shouldn't crash, but you're not meant to just make up your own layouts, if it didn't come from tmux it's effectively undefined behaviour :-).

I'll look at the change you're suggesting later.

--
You received this message because you are subscribed to the Google Groups "tmux-users" group.
To unsubscribe from this group and stop receiving emails from it, send an email to tmux-users+...@googlegroups.com.
To view this discussion, visit https://groups.google.com/d/msgid/tmux-users/CALDd4YWswm7yPofV9ip_-ksFfq14manCw%2BVf4XEuU2oWJ7-imw%40mail.gmail.com.

[Jere Viikari]

Definitely not the most important one but the only one found by GCC analyzers.
I'm running more scanners... :D

Thanks,
Jere

[Nicholas Marriott]

I have fixed this now, thanks.