Closed Bug 1449951 Opened 7 years ago Closed 6 years ago

Media libraries have questionable use of __declspec(safebuffers)

Categories

(Core :: Audio/Video: Playback, enhancement, P3)

enhancement

Tracking

()

RESOLVED WORKSFORME

People

(Reporter: away, Unassigned)

References

Details

(Keywords: sec-audit)

We fail Binscope.exe's GSFunctionSafeBuffers check because of: error BA2014: xul.dll is a C or C++ binary built with function(s) (InitCpuFlags,ArmCpuCaps,MipsCpuCaps) that disable the stack protector. The stack protector (/GS) is a security feature of the compiler which makes it more difficult to exploit stack buffer overflow memory corruption vulnerabilities. Disabling the stack protector, even on a function-by-function basis, is disallowed by SDL policy. To resolve this issue, remove occurrences of __declspec(safebuffers) from your code. If the additional code inserted by the stack protector has been shown in profiling to cause a significant performance problem for your application, attempt to move stack buffer modifications out of the hot path of execution to allow the compiler to avoid inserting stack protector checks in these locations rather than outright disabling the stack protector. It is not at all clear to me why InitCpuFlags,ArmCpuCaps,MipsCpuCaps are marked SAFEBUFFERS: https://searchfox.org/mozilla-central/search?q=SAFEBUFFERS&case=true&regexp=false&path= Init in particular is a one-time function and should not care about the perf hit of stack checks. Mips and Arm aren't even needed in current builds. Fortunately(?) these annotations are conditioned on !defined(__clang__), so if we let this bug sit around it will be fixed for free by changing to clang-cl.
Keywords: sec-audit
Can we have a quick look here for libvpx and libyuv ?
Rank: 25
Flags: needinfo?(rjesup)
Flags: needinfo?(jyavenard)
Priority: -- → P3
ArmCpuCaps/MipsCpuCaps don't seem to be used anywhere except within InitCpuFlags... InitCpuFlags is used, all over the place (via TestCpuFlags()), but appears will actually be called only once from TestCpuFlags() -- (cpu_flags_ ? InitCpuFlags() : cpu_flags_); InitCpuFlags() stores the result in cpu_flags_). It's used directly all the time in MaskCpuFlags, but that's only used in tests.
Flags: needinfo?(rjesup)
Flags: needinfo?(jyavenard)
Component: Audio/Video → Audio/Video: Playback

Fortunately(?) these annotations are conditioned on !defined(clang), so
if we let this bug sit around it will be fixed for free by changing to
clang-cl.

Hi from 2019.

Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.