mpv-player / mpv-player/mpv

Memory use after free in the WIP TA library

Open
#10,568 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
C
Stars
37k
Forks
3.5k
Avg merge
1d 10h
Merged PRs (30d)
22

Description

o/

Running some quick fuzzing, I noticed there is a use after free (in the master branch) using clang's static analyzer "scan-build". File is ta/ta.c at line 78.

mpv version and platform versions

Git master - git show commit 6c1f01d2841281fd594f8ce874b4113640c07553 (HEAD -> master, origin/master, origin/HEAD)

Reproduction steps

After installing build dependencies and scan-build otherwise I don't think meson generates a ninja target for it, (I'm on Ubuntu 22.04.1),

meson build -Db_sanitize=address
ninja -C build scan-build

The -Db_sanitize=address enables libasan, not necessary for this.

The scan-build target is made by meson, so I just invoke it. It reconfigures the meson project, builds it and uses clang's static analyzer.

The output I really am noticing here, because I don't feel like digging into the player for the other things is:

[209/244] Compiling C object mpv.p/ta_ta.c.o
../../../ta/ta.c:78:5: warning: Use of memory after it is freed [unix.Malloc]
    return h;
    ^~~~~~~~
../../../ta/ta.c:353:39: warning: Access to field 'leak_prev' results in a dereference of a null pointer (loaded from field 'leak_next') [core.NullDereference]
            cur->leak_next->leak_prev = cur->leak_prev;
                 ~~~~~~~~~            ^
2 warnings generated.
Expected behavior

I guess, for there to not be anything /shrug

Actual behavior
[209/244] Compiling C object mpv.p/ta_ta.c.o
../../../ta/ta.c:78:5: warning: Use of memory after it is freed [unix.Malloc]
    return h;
    ^~~~~~~~
../../../ta/ta.c:353:39: warning: Access to field 'leak_prev' results in a dereference of a null pointer (loaded from field 'leak_next') [core.NullDereference]
            cur->leak_next->leak_prev = cur->leak_prev;
                 ~~~~~~~~~            ^
2 warnings generated.

The ta_header struct calls return(h); - if ta_free() was called and the code jumps to the header, it is a UAF

Log file

So scan-build conveniently puts everything in a log, even into a set of HTML pages that can be viewed using scan-view.
2022-08-22-220138-87585-1.zip

other stuff

Was originally going to report with huntr for a CVE, but the README says you guys don't use ta in anything so I thought this would be more of a bug report

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with ta/ta.c at lines 78 and 353, then reproduce the analyzer output using meson build -Db_sanitize=address followed by ninja -C build scan-build. Trace the reported use-after-free and null dereference in the ta_header and leak-list paths; done means the scan-build warnings no longer appear without introducing new failures.

Written by the indexing model from the issue text.

Assessment

Tech stack
c
Domain
backend
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
38/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.