CVE-2023-40091 AI-generated analysis
platform/frameworks/base, cmds/incidentd (IncidentService binder shell-command handling)
The SHELL_COMMAND_TRANSACTION handler in incidentd's IncidentService::onTransact now reads the in/out/err file descriptors with Parcel::readUniqueFileDescriptor (which dups them) and hands owned fds to fdopen via release(), instead of fdopen'ing fds still owned by the Parcel.
Root cause
In cmds/incidentd/src/IncidentService.cpp, IncidentService::onTransact handled SHELL_COMMAND_TRANSACTION by calling data.readFileDescriptor() three times and passing the returned fds directly to fdopen(). Per the commit message, readFileDescriptor does not provide ownership of the fds while fdopen needs ownership, so the fds should have been duped. The old code did not check whether the fd reads succeeded. It also declared a local `int err` fd that shadowed the outer `status_t err`, and later overwrote it with the result of command().
How the fix works
- cmds/incidentd/src/IncidentService.cpp:IncidentService::onTransact (SHELL_COMMAND_TRANSACTION): replace the three data.readFileDescriptor() calls with unique_fd in, out, err and data.readUniqueFileDescriptor(&in/&out/&err). Per the commit message, readUniqueFileDescriptor dups the fds internally, so the handler owns its own descriptors.
- Each readUniqueFileDescriptor return status is checked. On any status != OK, the transaction returns that status immediately, before reading argc/args, the IShellCallback and the IResultReceiver.
- fdopen is now called with in.release(), out.release() and err.release(), which transfers ownership of the duped fds into the FILE* streams instead of using Parcel-owned fds.
- The result of command(fin, fout, ferr, args) is stored in a new status_t result and sent via resultReceiver->send(result). The `err` variable is no longer reused as both an fd and a status.
Completeness
- Covers all three fds (in, out, err) read in the SHELL_COMMAND_TRANSACTION case of IncidentService::onTransact; this is the only place the diff touches.
- The cleanup code after the diff (`if (fin != NULL) {` ...) is unchanged and only partially visible, so how the streams are closed is not shown.
- All three unique_fds are released before the fdopen NULL check. If an fdopen call fails, its released fd is no longer owned by a unique_fd, and the material does not show whether it is closed.
- Other incidentd entry points that may carry fds (for example AIDL-generated methods on IncidentService) are not shown and are not modified by this commit.
Behavior changes
- When a file descriptor cannot be read from the parcel, onTransact now returns the Parcel status immediately, without reading the remaining arguments or calling the result receiver.
- incidentd now works on duplicated fds (one dup per stream) rather than on the Parcel's own fds.
- No change to the shell command logic or the argument parsing is visible in the diff.
Per major
- Android 14: same fix: commit 29e8df07d8c4 in android-security-14.0.0_r3 (cherry-pick of ba78ef276951, Change-Id Ibe03a17dee91ac5bf25d123d4fd9c0bdd3c7d80e). The code shown at android-security-14.0.0_r29 is in the fixed form (readUniqueFileDescriptor + release()); the per-major facts report a 5/8 patched-region match there.
- Android 15: not affected per the bulletin; at android-security-15.0.0_r17, 5/8 lines of the patched region are present (same ratio as the fixed Android 14 tag); code not shown.
- Android 16: not affected per the bulletin; at android-security-16.0.0_r8, 5/8 lines of the patched region are present (same ratio as the fixed Android 14 tag); code not shown.
- Android 17: not affected per the bulletin; at android-security-17.0.0_r1, 5/8 lines of the patched region are present (same ratio as the fixed Android 14 tag); code not shown.
Unknowns
- Whether any permission or UID check gates SHELL_COMMAND_TRANSACTION in IncidentService::onTransact (not shown).
- The full cleanup code after the diff (closing of fin/fout/ferr) is only partially visible, including handling of a released fd whose fdopen failed.
- The concrete consequence of the ownership bug in the old code.
- Which commits, if any, applied this fix to Android 11, 12, 12L and 13 (listed as affected).
- Why only 5/8 patched-region lines match in the 14 to 17 tags.
- The contents of bug reports 286931110 and 283699145 (the commit's Test lines reference incidentd_service_fuzzer).