Skip to content

Latx fix config edge cases#303

Open
LaurenIsACoder wants to merge 1 commit into
lat-opensource:masterfrom
LaurenIsACoder:latx-fix-config-edge-cases
Open

Latx fix config edge cases#303
LaurenIsACoder wants to merge 1 commit into
lat-opensource:masterfrom
LaurenIsACoder:latx-fix-config-edge-cases

Conversation

@LaurenIsACoder
Copy link
Copy Markdown
Contributor

Summary

Fix two LATX string-handling boundary bugs and add a regression test.

Problem

Two boundary bugs can be triggered by ordinary runtime inputs.

  • target/i386/latx/latx-options.c:trim() skips leading whitespace and
    then unconditionally evaluates s + strlen(s) - 1. If the input is
    empty or contains only whitespace, this moves the pointer before the
    start of the buffer and may cause an out-of-bounds access.

    One practical trigger is a configuration entry with an empty value,
    for example:

    LATX_CLOSE_PARALLEL=
    

or

LATX_CLOSE_PARALLEL=

  • target/i386/latx/latx-special-args.c:extract_filename() truncates a
    long file name with strncpy(), but does not explicitly append '\0'.
    If the source name is longer than the destination buffer, the later
    strchr(buffer, '.') call may continue scanning past the end of the
    buffer.

Root Cause

  • trim() assumes that at least one character remains after leading
    whitespace is removed.

  • extract_filename() assumes that strncpy() always leaves a
    NUL-terminated destination string, which is not true for truncated
    copies.

Fix

  • Return early in trim() when the trimmed string is empty.
  • Stop the trailing-whitespace scan at the beginning of the string.
  • Handle NULL and zero-sized buffers in extract_filename().
  • Force NUL termination after strncpy() before scanning for '.'.
  • Add a regression test covering both cases.

Validation

Build

CC=cc CXX=c++ ./latxbuild/build64-dbg.sh

Build the regression test

cc
-DNEED_CPU_H
-DCONFIG_TARGET='"x86_64-linux-user-config-target.h"'
-DCONFIG_DEVICES='"x86_64-linux-user-config-devices.h"'
-fsanitize=address,undefined
-g -O1
-Ibuild64-dbg -I. -Iinclude -Itarget/i386/latx/include
$(pkg-config --cflags glib-2.0)
tests/latx-x86_64/latx-config-regression.c
-o build64-dbg/latx-config-regression
$(pkg-config --libs glib-2.0)

Run

LATX_AOT=0 ASAN_OPTIONS=detect_leaks=0
./build64-dbg/latx-config-regression

Result:

  • 5/5 tests passed on the fixed build.

Before the fix

Running the same regression binary against the pre-fix tree reports an
AddressSanitizer stack-buffer-underflow in trim().

Note on LATX_AOT=0 and ASAN_OPTIONS

  • LATX_AOT=0 keeps the validation on the runtime LATX path touched by
    this change, instead of mixing it with AOT behavior.

  • ASAN_OPTIONS=detect_leaks=0 only disables LeakSanitizer's leak check.
    It does not disable AddressSanitizer's out-of-bounds detection.

    This was added because LeakSanitizer may fail in some environments
    with a runtime/tooling issue such as ptrace, which is unrelated to
    the bugs fixed here. The actual boundary errors are still detected by
    AddressSanitizer normally.

Fix two non-AOT edge cases in the latx-x86_64 option path.

trim() used to walk before the start of an empty or all-whitespace
value when parsing entries such as LATX_CLOSE_PARALLEL=.
extract_filename() also relied on strncpy() without forcing NUL
termination, so long program names could trigger an out-of-bounds
read during extension scanning.

Add a regression test covering both cases with LATX_AOT=0.

Signed-off-by: Hanlu Li <heuleehanlu@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant