input: bound input_remap_ids[] against analog_value[][8] OOB write

LibretroAdmin · LibretroAdmin · commit cba3a9c909de · 2026-04-28T03:26:06.000+02:00
input_driver.c's poll-time remap loop indexed
mapper-&gt;analog_value[i][n] using values from
settings-&gt;uints.input_remap_ids[i][k] without proper bounds in
two places:

  1. Button -&gt; analog branch (line ~7392):
        handle-&gt;analog_value[i][remap_button - RARCH_FIRST_CUSTOM_BIND] = ...
     No bound on remap_button at all.  Reachable values:
     [RARCH_FIRST_CUSTOM_BIND, RARCH_UNMAPPED) i.e. [16, 1024),
     producing remap_axis_bind in [0, 1008).  Indexing past the
     8-element row writes int16_t values up to ~2014 bytes past
     the end of analog_value, into adjacent input_mapper_t
     fields (keys[], key_button[], buttons[]) and beyond into
     adjacent input_driver_state_t fields.

  2. Analog -&gt; analog branch (line ~7427):
        if (remap_axis_bind &lt; sizeof(handle-&gt;analog_value[i]))
           handle-&gt;analog_value[i][remap_axis_bind] = ...
     The sizeof() returns 16 (the byte size of the int16_t[8]
     row), not the element count.  Indices 8..15 passed the
     check and OOB-wrote into the next user's analog_value row,
     or past the array entirely for the last user.

Both sites read remap_button / remap_axis from
settings-&gt;uints.input_remap_ids[i][k], which is loaded from
.rmp files via input_remapping_load_file in configuration.c.
Pre-this-patch the loader fed config_get_int's result directly
into storage with no range check, so a malformed .rmp could
plant any int there.  An attacker who can deliver a .rmp
alongside a ROM (a common distribution channel for
"controller config bundles") gets a 60Hz attacker-controlled
out-of-bounds write into adjacent input state -- including
the keys[] keyboard bitmask for the last user, which controls
which RetroArch hotkeys appear pressed.

Two-pronged fix:

  - Load-time validation in input_remapping_load_file:
    reject _remap values outside [0, RARCH_ANALOG_BIND_LIST_END)
    (clamping to RARCH_UNMAPPED) before storing into
    settings-&gt;uints.input_remap_ids[i][j].  Applied to both the
    button-branch (j &lt; RARCH_FIRST_CUSTOM_BIND) and the
    analog-branch (j &gt;= RARCH_FIRST_CUSTOM_BIND) of the
    loader.

  - Use-site bounds in input_driver.c: add an
    ARRAY_SIZE-based bound to the previously-unguarded
    button -&gt; analog branch, and replace the broken sizeof()
    bound with ARRAY_SIZE() in the analog -&gt; analog branch.
    Defence in depth: even if the load-time validator misses
    a path (or a future caller writes to input_remap_ids[]
    bypassing the loader), the use sites are now safe.

Adds samples/tasks/input_remap/input_remap_bounds_test as a
regression test, registered in
.github/workflows/Linux-samples-tasks.yml as an ASan-enabled
step.  Seven cases covering the load-time clamp's accept and
reject paths, in-range writes going to the correct slot,
out-of-range writes being skipped without OOB, and the
specific historical sizeof-vs-ARRAY_SIZE bug shape.

Test follows the existing samples/tasks pattern (verbatim copy
of the post-fix predicates with a maintenance-contract comment;
ASan as the discriminator).  Verified to fire under ASan when
the bound check is removed: heap-buffer-overflow WRITE of size
2 at offset 16 of a 16-byte allocation, in
do_button_to_analog_write.  With the bound the test passes
clean.
diff --git a/.github/workflows/Linux-samples-tasks.yml b/.github/workflows/Linux-samples-tasks.yml
@@ -126,3 +126,27 @@ jobs:
           test -x video_shader_wildcard_test
           timeout 60 ./video_shader_wildcard_test
           echo "[pass] video_shader_wildcard_test"
+
+      - name: Build and run input_remap_bounds_test (ASan)
+        shell: bash
+        working-directory: samples/tasks/input_remap
+        run: |
+          set -eu
+          # Regression test for the input-remap analog-axis OOB-
+          # write fixes in configuration.c::input_remapping_load_file()
+          # and input/input_driver.c.  Pre-fix a malformed .rmp
+          # could supply a remap target outside [0, RARCH_ANALOG_
+          # BIND_LIST_END), which the use sites in input_driver.c
+          # then indexed into a fixed-size analog_value[][8] array
+          # without bounds (button -> analog branch) or with a
+          # broken bound that compared elements against bytes
+          # (analog -> analog branch using sizeof instead of
+          # ARRAY_SIZE).  Build under AddressSanitizer so any
+          # reintroduction of either primitive is caught at the
+          # bounds level.  If configuration.c or input_driver.c
+          # amends the relevant predicates, the verbatim copies
+          # in input_remap_bounds_test.c must follow.
+          make clean all SANITIZER=address
+          test -x input_remap_bounds_test
+          timeout 60 ./input_remap_bounds_test
+          echo "[pass] input_remap_bounds_test"
diff --git a/configuration.c b/configuration.c
@@ -6984,6 +6984,21 @@ bool input_remapping_load_file(void *data, const char *path)
                if (_remap == -1)
                   _remap = RARCH_UNMAPPED;
 
+               /* Reject .rmp values outside the legitimate range.
+                * Runtime use sites in input_driver.c index a
+                * fixed-size analog_value[][8] array using
+                * (_remap - RARCH_FIRST_CUSTOM_BIND); pre-this-patch
+                * a malformed .rmp could supply an arbitrary int
+                * here and OOB-write up to ~1007 elements past
+                * analog_value, corrupting adjacent input-state
+                * fields and (for the last user) the keys[] mask.
+                * Legal values are any concrete bind index in
+                * [0, RARCH_ANALOG_BIND_LIST_END) plus the
+                * RARCH_UNMAPPED sentinel. */
+               if (   _remap != (int)RARCH_UNMAPPED
+                   && (_remap < 0 || _remap >= (int)RARCH_ANALOG_BIND_LIST_END))
+                  _remap = RARCH_UNMAPPED;
+
                configuration_set_uint(settings,
                      settings->uints.input_remap_ids[i][j], _remap);
             }
@@ -7012,6 +7027,12 @@ bool input_remapping_load_file(void *data, const char *path)
                if (_remap == -1)
                   _remap = RARCH_UNMAPPED;
 
+               /* See the matching comment in the button-branch
+                * above: same OOB-write primitive, same fix. */
+               if (   _remap != (int)RARCH_UNMAPPED
+                   && (_remap < 0 || _remap >= (int)RARCH_ANALOG_BIND_LIST_END))
+                  _remap = RARCH_UNMAPPED;
+
                configuration_set_uint(settings,
                      settings->uints.input_remap_ids[i][j], _remap);
             }
diff --git a/input/input_driver.c b/input/input_driver.c
@@ -7384,13 +7384,26 @@ void input_driver_poll(void)
                      }
                      else
                      {
-                        int invert = 1;
+                        unsigned remap_axis_bind =
+                              remap_button - RARCH_FIRST_CUSTOM_BIND;
+                        int      invert          = 1;
+
+                        /* Bound: analog_value is int16_t[8].  remap_button
+                         * comes from settings->uints.input_remap_ids[][]
+                         * which is loaded from .rmp config files;
+                         * input_remapping_load_file's config_get_int
+                         * accepts any integer, so a malformed .rmp can
+                         * supply remap_button anywhere in (RARCH_FIRST_
+                         * CUSTOM_BIND, INT_MAX], producing a write at
+                         * arbitrary offset past analog_value[i].  Reject
+                         * out-of-range targets here. */
+                        if (remap_axis_bind >= ARRAY_SIZE(handle->analog_value[i]))
+                           continue;
 
                         if (remap_button % 2 != 0)
                            invert = -1;
 
-                        handle->analog_value[i][
-                           remap_button - RARCH_FIRST_CUSTOM_BIND] =
+                        handle->analog_value[i][remap_axis_bind] =
                               (p_new_state->analog_buttons[j]
                                ? p_new_state->analog_buttons[j]
                                : 32767) * invert;
@@ -7424,7 +7437,11 @@ void input_driver_poll(void)
                         unsigned remap_axis_bind =
                            remap_axis - RARCH_FIRST_CUSTOM_BIND;
 
-                        if (remap_axis_bind < sizeof(handle->analog_value[i]))
+                        /* Pre-patch this read 'sizeof(handle->analog_value[i])'
+                         * which is 16 (bytes), but the array has 8 elements;
+                         * any remap_axis_bind in [8, 15] caused an OOB write.
+                         * Use ARRAY_SIZE so the check is in elements. */
+                        if (remap_axis_bind < ARRAY_SIZE(handle->analog_value[i]))
                         {
                            int invert = 1;
                            if (     (k % 2 == 0 && remap_axis % 2 != 0)
diff --git a/samples/tasks/input_remap/Makefile b/samples/tasks/input_remap/Makefile
@@ -0,0 +1,25 @@
+TARGET := input_remap_bounds_test
+
+SOURCES := input_remap_bounds_test.c
+
+OBJS := $(SOURCES:.c=.o)
+
+CFLAGS += -Wall -pedantic -std=gnu99 -g -O0
+
+ifneq ($(SANITIZER),)
+   CFLAGS  := -fsanitize=$(SANITIZER) -fno-omit-frame-pointer $(CFLAGS)
+   LDFLAGS := -fsanitize=$(SANITIZER) $(LDFLAGS)
+endif
+
+all: $(TARGET)
+
+%.o: %.c
+	$(CC) -c -o $@ $< $(CFLAGS)
+
+$(TARGET): $(OBJS)
+	$(CC) -o $@ $^ $(LDFLAGS)
+
+clean:
+	rm -f $(TARGET) $(OBJS)
+
+.PHONY: clean
diff --git a/samples/tasks/input_remap/input_remap_bounds_test.c b/samples/tasks/input_remap/input_remap_bounds_test.c
