MPEG-H MpeghUiManagerJni.command() truncates non-ASCII UI actions and leaks its string buffer on every call #3365

Version

Media3 main branch

More version details

Reviewed at commit 5fb306449733dd71595700c1227ad6087578c559 (release branch, "Bump version to 1.10.1").

The affected file is byte-for-byte identical at the current release head (2bc207851df311340767e913931ca7b28cab1794, version 1.11.0), so the report applies to 1.10.1 and 1.11.0 alike.

Devices that reproduce the issue

Not device-specific. The defect is in the JNI layer of the decoder_mpegh module and is independent of SoC, Android version and audio HAL. Any device running an app that builds in decoder_mpegh and sends an MPEG-H UI command is affected.

Devices that do not reproduce the issue

No response

Reproducible in the demo app?

Not tested

Reproduction steps

1. Build the decoder_mpegh module following libraries/decoder_mpegh/README.md (clone mpeghdec at r3.0.2 into libraries/decoder_mpegh/src/main/jni/libmpegh).
2. Play an MPEG-H stream (MimeTypes.AUDIO_MPEGH_MHM1) through MpeghAudioRenderer.
3. Send a UI command to the renderer:

player
    .createMessage(mpeghAudioRenderer)
    .setType(Renderer.MSG_SET_CODEC_PARAMETERS)
    .setPayload(
        new CodecParameters.Builder()
            // any XML action; use one whose payload contains a non-ASCII character
            .setParameter(MpeghAudioRenderer.CODEC_PARAM_MPEGH_UI_COMMAND, xmlAction)
            .build())
    .send();

That reaches the affected code as:

MpeghAudioRenderer.handleMessage() (line 165) → MpeghUiCommandHelper.addCommand() → drained in MpeghDecoder.decode() (lines 154 and 169) → MpeghUiManagerJni.command(String) → mpeghuimanager_jni.cpp line 172.

4. For the truncation symptom, make xmlAction contain a non-ASCII character in a position where truncation is observable — a preset or group label such as Kommentar Deutsch (Originalfassung) or any label with a CJK character. For the leak symptom, send commands repeatedly and watch native heap growth.

Expected result

• The XML action is applied in full, whatever characters it contains.
• The character buffer obtained from GetStringUTFChars() is released, so repeated commands do not grow the native heap.
• An allocation failure surfaces as a Java exception rather than a native crash.

Actual result

The whole function is nine lines and contains three separate problems:

UIMANAGER_FUNC(jboolean, command, jstring xmlAction) {
  UIMANAGER_CONTEXT* ctx = getContext_UI(env, obj);

  const char* str = env->GetStringUTFChars(xmlAction, 0);
  uint32_t strlen = env->GetStringLength(xmlAction);

  unsigned int flagsOut = 0;
  MPEGH_UI_ERROR result =
      mpegh_UI_ApplyXmlAction(ctx->handle, str, strlen, &flagsOut);
  if (result != MPEGH_UI_OK) {
    LOGW("Failed to apply XML action with result %d for command %s", result,
         str);
    return false;
  }
  return true;
}

1. A UTF-16 length is passed with a modified-UTF-8 buffer

GetStringUTFChars() returns modified-UTF-8 bytes, but GetStringLength() returns the number of UTF-16 code units in the Java string. The two agree only for strings made entirely of ASCII characters.

mpegh_UI_ApplyXmlAction() documents its third parameter as Size of the XML string (number of chars) (mpeghUIManager.h), i.e. a byte count over the buffer it was handed. For any XML action containing a non-ASCII character the length passed is smaller than the byte length of str, so the action is silently truncated mid-document; for characters outside the Basic Multilingual Plane the relationship inverts and the length overruns the intended content.

XML actions carry user-facing strings such as preset and group labels, so non-ASCII content is reachable through ordinary use rather than only through malformed input.

GetStringUTFLength() is the call that matches a GetStringUTFChars() buffer.

2. The character buffer is never released

Every successful GetStringUTFChars() must be paired with ReleaseStringUTFChars(); returning from the native method does not release it implicitly. There is no release anywhere in this function, so the acquisition leaks on the error path and on every ordinary successful call.

On ART GetStringUTFChars() allocates a copy, so each call leaks that copy for the lifetime of the process. MpeghDecoder.decode() drains the pending command queue on every access unit that carries commands, so the leak accumulates with UI activity rather than only under error conditions.

3. The GetStringUTFChars() result is not checked

GetStringUTFChars() returns NULL with an OutOfMemoryError pending when it cannot allocate the copy. str is then handed to mpegh_UI_ApplyXmlAction(), and on the failure branch to LOGW() as a %s argument, both of which dereference it. The function also never calls ExceptionCheck(), so it returns a normal jboolean with an exception already pending.

Suggested fix

All three fold into one rewrite:

UIMANAGER_FUNC(jboolean, command, jstring xmlAction) {
  UIMANAGER_CONTEXT* ctx = getContext_UI(env, obj);

  const char* str = env->GetStringUTFChars(xmlAction, 0);
  if (str == nullptr) {
    return false;  // exception already pending
  }
  uint32_t strlen = env->GetStringUTFLength(xmlAction);

  unsigned int flagsOut = 0;
  MPEGH_UI_ERROR result =
      mpegh_UI_ApplyXmlAction(ctx->handle, str, strlen, &flagsOut);
  if (result != MPEGH_UI_OK) {
    LOGW("Failed to apply XML action with result %d for command %s", result, str);
  }
  env->ReleaseStringUTFChars(xmlAction, str);
  return result == MPEGH_UI_OK;
}

An RAII wrapper around the acquisition would keep any further early returns covered.

Media

Not applicable to the defect itself — the truncation and the leak are in the handling of the String argument to command(), and are independent of the media being decoded.

To exercise the path at runtime, any MPEG-H stream with MimeTypes.AUDIO_MPEGH_MHM1 will do; the trigger is the UI command sent alongside it, not the stream content.

Bug Report

• You will email the zip file produced by adb bugreport to android-media-github@google.com after filing this issue.