Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix excessive stack usage when calling vorbis_analysis_wrote with lots of samples #104

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Commits on Nov 26, 2023

  1. Fix excessive stack usage when calling vorbis_analysis_wrote with l…

    …ots of samples
    
    `vorbis_analysis_wrote` increments `v->pcm_current` by `vals`, and this
    incremented value can be used by `_preextrapolate_helper` right after to
    allocate a float array in the stack `v->pcm_current` positions large.
    Clearly, since `alloca` does not check that there is enough stack space
    available to satisfy the allocation request, this can lead to a stack
    overflow and memory corruption, which at best have no effect, more
    likely cause segmentation faults, and at worst introduce security risks.
    
    The documentation for `vorbis_analysis_buffer` and
    `vorbis_analysis_wrote` does not specify a maximum value for `vals`. It
    states that "1024 is a reasonable choice", but callers are free to use
    larger or smaller counts as they wish. Therefore, `libvorbis` not
    handling this case is undesirable behavior.
    
    To better handle this case without throwing the performance benefits of
    `alloca` out the window, let's check whether the allocation would exceed
    256 KiB (an estimate for the minimum stack space available is 1 MiB,
    which is [the default on Windows
    platforms](https://learn.microsoft.com/en-us/windows/win32/procthread/thread-stack-size)),
    and if so fall back to a heap allocated array. The heap array that may
    be allocated for this purpose is freed when `vorbis_dsp_clear` is
    called. `_preextrapolate_helper` takes neglible execution time
    compared to the encoding process for usual sample block sizes, though.
    
    Signed-off-by: Alejandro González <[email protected]>
    AlexTMjugador committed Nov 26, 2023
    Configuration menu
    Copy the full SHA
    f3c156f View commit details
    Browse the repository at this point in the history