Closed Bug 958988 Opened 8 years ago Closed 8 years ago

OpenH264: stack-buffer-overflow crash [@WelsSVCEnc::RcInitTlWeight]

Categories

(Core :: WebRTC: Audio/Video, defect)

x86_64
macOS
defect
Not set
critical

Tracking

()

RESOLVED FIXED
Tracking Status
firefox-esr24 --- unaffected
b2g18 --- unaffected
b2g-v1.2 --- unaffected
b2g-v1.3 --- unaffected

People

(Reporter: posidron, Unassigned)

References

(Blocks 1 open bug)

Details

(5 keywords)

Attachments

(3 files)

codec/encoder/core/src/ratectl.cpp:196

void RcInitTlWeight (sWelsEncCtx* pEncCtx) {
  SWelsSvcRc* pWelsSvcRc = &pEncCtx->pWelsSvcRc[pEncCtx->uiDependencyId];
  SRCTemporal* pTOverRc = pWelsSvcRc->pTemporalOverRc;
  SDLayerParam* pDLayerParam =  &pEncCtx->pSvcParam->sDependencyLayers[pEncCtx->uiDependencyId];
  const int32_t kiDecompositionStages = pDLayerParam->iDecompositionStages;
  const int32_t kiHighestTid = pDLayerParam->iHighestTemporalId;

  [...]
  
  n = 0;
  while (n <= kiHighestTid) {
*   pTOverRc[n].dTlayerWeight   = WeightArray[kiDecompositionStages][n];
    ++ n;
  }


To reproduce do: 

  ./h264enc welsenc_fuzz.cfg

Happened accidentally while auto generating a new configuration.

Tested with https://github.com/cisco/openh264/commit/4a8a9aabc1
Attached file welsenc_fuzz.cfg
Attached file layer1_fuzz.cfg
Attached file callstack
Taking a quick look at this, it appears that this may be a bogus configuration. Which isn't to say it should crash :)
I am yet unclear about what all these flags exactly do but I believe it is related to the GOPSize entry.
Using 16 here crashes, whereby 4 makes no trouble.
Component: Video/Audio → WebRTC: Audio/Video
I checked code, there is no overflows for "WeightArray" variable.

is it a false positive?  I found the blew hint when running code.

" HINT: this may be a false positive if your program uses some custom stack unwind mechanism or swapcontext
      (longjmp and C++ exceptions *are* supported)"
(In reply to karina from comment #6)
> is it a false positive?  I found the blew hint when running code.

Seems possible, "kiHighestTid" and "kiDecompositionStages" are both 4.

Can we add some code to prevent ASan from complaining about it?
Wait. It's late here. Shouldn't that be:

while (n < kiHighestTid) {
  ...
}
got it. I found the root cause for this.

there is some potential issues to support GOPsize = 16. We will discuss whether we close GOPsize = 16.  
will update it later. thanks for you help.
Hi Christoph:
   fix it. Could you verify it on cisco/master branch? thanks!
Fixed.

Tested with https://github.com/cisco/openh264/commit/de935a523
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Group: core-security → core-security-release
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.