mj_recompile reads past saved actuator state when an actuator's actdim increases

Intro

Hi!

I am testing state continuity across live mjSpec schema changes. I first sent this finding to Google's vulnerability intake, and they advised reporting it as an issue in the MuJoCo repository.

My setup

• MuJoCo 3.13.0 at 123347c0eeab7e13c8da0828ab593bbd95bcf335
• MuJoCo main at 10124d5d9dca411ec3c8988aa1e3b619103d71bb
• C API from a minimal C++17 client, double precision
• Ubuntu 24.04.5 LTS, Linux 7.0.0-28-generic x86_64
• GCC / G++ 13.3.0
• Normal build and whole-library AddressSanitizer build

What's happening? What did you expect?

Expanding a surviving actuator's activation dimension before calling mj_recompile makes state restoration read past the end of the saved activation vector.

The reproducer initially compiles a general actuator with dyntype="integrator", giving it one activation value. It then changes the same actuator to dyntype=mjDYN_USER, sets actdim=3, and calls mj_recompile.

I expected recompilation to do one of the following safely: reject the incompatible transition, reset the actuator state, or preserve only the compatible old component and initialize the two added components. It should not read beyond the state that was saved.

Instead, SaveState stores one mjtNum using the old actdim_. Compilation changes actdim_ to 3, and RestoreState then passes that new dimension to mjuu_copyvec while reading from the one-element saved vector. AddressSanitizer reports a 24-byte read immediately after an 8-byte allocation.

On release 3.13.0:

ERROR: AddressSanitizer: heap-buffer-overflow
READ of size 24
#7 mjCModel::RestoreState ... src/user/user_model.cc:4147
#8 mj_recompile ... src/user/user_api.cc:340
0x5020000013f8 is located 0 bytes after 8-byte region

Current main produces the same result at user_model.cc:4154. An uninstrumented build returned success and printed the following on my validation host:

result=0 na=3 act=42,0,0

The two trailing values are allocator-dependent; this output is not the correctness oracle. The sanitizer report is the deterministic evidence of the invalid source read.

Relevant source on release 3.13.0:

• actdim_ is replaced during compilation
• SaveState sizes the saved vector with the old activation dimension
• RestoreState copies using the new activation dimension

The corresponding current-main save and restore locations are user_model.cc:4097-4102 and user_model.cc:4150-4155.

This is related to, but distinct from, #3586. That issue reads the old qpos / qvel arrays during SaveState using a mutated joint width. This issue saves the old actuator activation width, then reads past that saved vector during RestoreState using the newly compiled actdim_.

Steps for reproduction

1. Check out MuJoCo 3.13.0 at 123347c0eeab7e13c8da0828ab593bbd95bcf335.
2. Save the code below as /tmp/recompile-actdim-reproducer.cc.
3. From the MuJoCo checkout, run:

cmake -S . -B build-actdim-asan \
  -DCMAKE_BUILD_TYPE=Debug \
  -DCMAKE_C_COMPILER=gcc-13 \
  -DCMAKE_CXX_COMPILER=g++-13 \
  -DCMAKE_C_FLAGS='-fsanitize=address -fno-omit-frame-pointer' \
  -DCMAKE_CXX_FLAGS='-fsanitize=address -fno-omit-frame-pointer' \
  -DCMAKE_SHARED_LINKER_FLAGS='-fsanitize=address' \
  -DMUJOCO_BUILD_EXAMPLES=OFF \
  -DMUJOCO_BUILD_SIMULATE=OFF \
  -DMUJOCO_BUILD_TESTS=OFF
cmake --build build-actdim-asan --target mujoco --parallel
g++-13 -std=c++17 -O0 -g -fsanitize=address -fno-omit-frame-pointer \
  -Iinclude /tmp/recompile-actdim-reproducer.cc \
  -Lbuild-actdim-asan/lib -Wl,-rpath,'$ORIGIN/lib' \
  -lmujoco -o build-actdim-asan/reproducer
ASAN_OPTIONS=detect_leaks=0:halt_on_error=1 \
  build-actdim-asan/reproducer

1. Observe the 24-byte heap-buffer-overflow read in mjCModel::RestoreState.

The attached recompile-actdim-heap-overread-poc.zip contains the same source, commands, environment information, and complete sanitizer traces for 3.13.0 and current main.

Minimal model for reproduction

The complete asset-free MJCF is embedded in the program below. It contains one body, one hinge joint, one sphere, and one actuator.

Code required for reproduction

#include <cstdio>
#include <cstdlib>

#include <mujoco/mujoco.h>

int main() {
  static constexpr char xml[] = R"(
  <mujoco>
    <worldbody>
      <body>
        <joint name="joint" type="hinge"/>
        <geom type="sphere" size=".1" mass="1"/>
      </body>
    </worldbody>
    <actuator>
      <general name="actuator" joint="joint" dyntype="integrator"/>
    </actuator>
  </mujoco>)";

  char error[1024] = {};
  mjSpec* spec = mj_parseXMLString(xml, nullptr, error, sizeof(error));
  if (!spec) {
    std::fprintf(stderr, "parse failed: %s\n", error);
    return EXIT_FAILURE;
  }
  mjModel* model = mj_compile(spec, nullptr);
  mjData* data = model ? mj_makeData(model) : nullptr;
  if (!model || !data) {
    std::fprintf(stderr, "initial compile failed: %s\n", mjs_getError(spec));
    return EXIT_FAILURE;
  }

  data->act[0] = 42.0;
  mjsActuator* actuator =
      mjs_asActuator(mjs_findElement(spec, mjOBJ_ACTUATOR, "actuator"));
  actuator->dyntype = mjDYN_USER;
  actuator->actdim = 3;

  const int result = mj_recompile(spec, nullptr, model, data);
  std::printf("result=%d na=%td act=", result, model->na);
  for (int i = 0; i < model->na; ++i) {
    std::printf("%s%.17g", i ? "," : "", data->act[i]);
  }
  std::printf("\n");

  mj_deleteData(data);
  mj_deleteModel(model);
  mj_deleteSpec(spec);
  return result == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
}

I have not included a speculative production patch because changing both activation width and dynamics type may require resetting the actuator state rather than copying a prefix. A safe implementation must retain the saved cardinality, never copy beyond it, and define the compatibility policy for changed activation semantics.

Confirmations

• I searched the latest documentation thoroughly before posting.
• I searched previous Issues and Discussions, I am certain this has not been raised before.