Skip to content

cursor: Fix handling of exotic cursor files

Tobias Stoeckmann requested to merge tstoeckmann/wayland:cursor into main

This merge request fixes a few undefined behaviors and possible memory corruption with invalid cursor files.

I have a few proof of concepts for these commits:

Commit 1: Undefined behavior with huge names

If an index.theme contains a theme name which gets close to INT_MAX, then creation of full path can lead to a signed integer overflow, which is undefined behavior.

Fix this by turning one of the values to size_t. Easy solution for a probably never occurring issue.

Proof of Concept:

  1. Compile wayland with -Bb_sanitize=undefined

  2. Create huge index.theme

MYBASE=$(mktemp -d)
mkdir -p $MYBASE/default
dd if=/dev/zero bs=4096 count=524288 | tr '\0' A > $MYBASE/default/index.theme
echo -n Inherits= | dd bs=1 of=$MYBASE/default/index.theme conv=notrunc
  1. Run weston with default settings
XCURSOR_PATH=$MYBASE weston --no-config

You can see a message like this:

runtime error: signed integer overflow

Commit 2: Gracefully handle out of memory condition

If the full path could not be constructed, avoid calling opendir(NULL) which, depending on library, might trigger undefined behavior.

Proof of Concept: re-run proof of concept of previous commit

The output will be something like:

runtime error: null pointer passed as argument 1, which is declared to never be null

Commit 3: Gracefully handle huge cursor files

If cursor files require more than INT_MAX bytes, it is possible to trigger out of boundary writes.

Since these sizes are most likely not desired anyway, gracefully handle these situations like out of memory errors.

Proof of Concept:

  1. Create file with maximum sized cursor
MYBASE=$(mktemp -d)
mkdir -p $MYBASE/default/cursors
base64 -d <<< WGN1chAAAAAAAAAAAQAAAAIA/f//fwAAHAAAAAAAAAACAP3//38AAAAAAAD/fwAA/38= > $MYBASE/default/cursors/dnd-move
dd if=/dev/zero of=$MYBASE/default/cursors/dnd-move bs=4 count=1 seek=1073676305 conv=notrunc
  1. Create weston.ini
cat > $MYBASE/weston.ini << EOF
[shell]
cursor-size=16000
EOF
  1. Start weston
XCURSOR_PATH=$MYBASE weston -c $MYBASE/weston.ini

Eventually weston crashes for overwriting data outside of mmapped areas. Ideally, compile wayland and weston with -Db_sanitize=address. If you do, you might have to use ASAN_OPTIONS=detect_odr_violation=0 when starting weston like I did.

Commit 4: Ignore invalid cursor files

This has been merged upstream yesterday, see xorg/lib/libxcursor@833735e3

Commit 5: Properly check realloc result

Otherwise we leave the structure in an inconsistent state

Merge request reports