Issue metadata
Sign in to add a comment
|
Security: PDFium (XFA) Heap Buffer Overflow in CWeightTable::Calc
Reported by
stackexp...@gmail.com,
Oct 8 2016
|
||||||||||||||||||||||
Issue descriptionVULNERABILITY DETAILS Issue 619398 was not fixed properly but labeled as verified and security view restrictions have been removed. So I opened this new issue. The proof-of-concept files for Issue 619398 have been deleted since it still can be reproducible. The root cause of this issue is that the bounds check in function CWeightTable::Calc was insufficient. There were two patches related to the bounds check. 1. Lei Zhang (@thestig) submitted a patch for this issue at https://pdfium.googlesource.com/pdfium/+/766901f5ec79b3c3ccd1e872f699642d771a89c5 on Aug 04. 2. I (Ke Liu, @stackexploit) submitted another patch to strengthen the bounds check at https://pdfium.googlesource.com/pdfium/+/5aed0216ad6574944e76a95ef0dbbc910bab4a1a on Sep 26. Today I found that my patch (submitted on Sep 26) was insufficient too. The beginning address of array |PixelWeight.m_Weights| can be changed dynamically. So we must calculate the array size dynamically too. struct PixelWeight { int m_SrcStart; int m_SrcEnd; int m_Weights[1]; // ----> The beginning address of this array can be changed dynamically }; 44 size_t CWeightTable::GetPixelWeightSize() const { 45 return m_dwWeightTablesSize / sizeof(int); // ----> Must be calculated dynamically 46 } 47 The solution is easy. The size of heap block |m_pWeightTables| is |m_dwWeightTablesSize| so we can get the end address of the it. Also, we can get the beginning address of array |PixelWeight.m_Weights| directly. Then we can calculate the maximum array size dynamically. size_t CWeightTable::GetMaximumPixelWeightSize(PixelWeight* pWeight) const { uint8_t* end_addr = m_pWeightTables + m_dwWeightTablesSize; uint8_t* begin_addr = reinterpret_cast<uint8_t*>(&pWeight->m_Weights); return (end_addr - begin_addr) / sizeof(pWeight->m_Weights[0]); } VERSION Chrome Version: PDFium with XFA enabled Operating System: All REPRODUCTION CASE See attachments. FOR CRASHES, PLEASE INCLUDE THE FOLLOWING ADDITIONAL INFORMATION Type of crash: tab Crash State: ==21787==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x7fffa0f20704 at pc 0x0000020d090f bp 0x7fffffffd140 sp 0x7fffffffd138 WRITE of size 4 at 0x7fffa0f20704 thread T0 #0 0x20d090e in CWeightTable::Calc(int, int, int, int, int, int, int) core/fxge/dib/fx_dib_engine.cpp:245:36 #1 0x20d6773 in CStretchEngine::StartStretchHorz() core/fxge/dib/fx_dib_engine.cpp:418:21 #2 0x20d83d6 in CFX_ImageStretcher::StartStretch() core/fxge/dib/fx_dib_engine.cpp:937:21 #3 0x20d7932 in CFX_ImageStretcher::Start() core/fxge/dib/fx_dib_engine.cpp:924:10 #4 0x2080a74 in CFX_ImageRenderer::Start(CFX_DIBitmap*, CFX_ClipRgn const*, CFX_DIBSource const*, int, unsigned int, CFX_Matrix const*, unsigned int, int, int, void*, int) core/fxge/dib/fx_dib_main.cpp:1606:23 #5 0x20c294b in CFX_AggDeviceDriver::StartDIBits(CFX_DIBSource const*, int, unsigned int, CFX_Matrix const*, unsigned int, void*&, int) core/fxge/agg/fx_agg_driver.cpp:1706:14 #6 0x25f5113 in CXFA_ImageRenderer::StartDIBSource() xfa/fxfa/app/xfa_ffwidget.cpp:626:18 #7 0x25f7986 in Start xfa/fxfa/app/xfa_ffwidget.cpp:622:10 #8 0x25f7986 in XFA_DrawImage(CFX_Graphics*, CFX_RTemplate<float> const&, CFX_Matrix*, CFX_DIBitmap*, int, int, int, int, int) xfa/fxfa/app/xfa_ffwidget.cpp:899 #9 0x27b1301 in CXFA_FFImage::RenderWidget(CFX_Graphics*, CFX_Matrix*, unsigned int) xfa/fxfa/app/xfa_ffimage.cpp:63:5 #10 0x2614dd2 in CXFA_RenderContext::DoRender(IFX_Pause*) xfa/fxfa/app/xfa_rendercontext.cpp:60:16 #11 0x1c4b41d in CPDFSDK_PageView::PageView_OnDraw(CFX_RenderDevice*, CFX_Matrix*, CPDF_RenderOptions*, FX_RECT const&) fpdfsdk/cpdfsdk_pageview.cpp:112:21 #12 0x1c343ec in FFLCommon fpdfsdk/fpdfformfill.cpp:132:16 #13 0x1c343ec in FPDF_FFLDraw fpdfsdk/fpdfformfill.cpp:407 #14 0x4f5da5 in RenderPage(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, void*, void*&, FPDF_FORMFILLINFO_PDFiumTest&, int, Options const&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&) samples/pdfium_test.cc:594:5 #15 0x4f735d in RenderPdf(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, char const*, unsigned long, Options const&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&) samples/pdfium_test.cc:786:9 #16 0x4f8878 in main samples/pdfium_test.cc:916:5 #17 0x7ffff69d382f in __libc_start_main /build/glibc-GKVZIf/glibc-2.23/csu/../csu/libc-start.c:291 0x7fffa0f20704 is located 0 bytes to the right of 335552260-byte region [0x7fff8cf1e800,0x7fffa0f20704) allocated by thread T0 here: #0 0x4c2573 in __interceptor_calloc (/pdfium/out/Debug/pdfium_test+0x4c2573) #1 0x20cf31d in CWeightTable::Calc(int, int, int, int, int, int, int) core/fxge/dib/fx_dib_engine.cpp:70:21 #2 0x20d6773 in CStretchEngine::StartStretchHorz() core/fxge/dib/fx_dib_engine.cpp:418:21 #3 0x20d83d6 in CFX_ImageStretcher::StartStretch() core/fxge/dib/fx_dib_engine.cpp:937:21 #4 0x20d7932 in CFX_ImageStretcher::Start() core/fxge/dib/fx_dib_engine.cpp:924:10 #5 0x2080a74 in CFX_ImageRenderer::Start(CFX_DIBitmap*, CFX_ClipRgn const*, CFX_DIBSource const*, int, unsigned int, CFX_Matrix const*, unsigned int, int, int, void*, int) core/fxge/dib/fx_dib_main.cpp:1606:23 #6 0x20c294b in CFX_AggDeviceDriver::StartDIBits(CFX_DIBSource const*, int, unsigned int, CFX_Matrix const*, unsigned int, void*&, int) core/fxge/agg/fx_agg_driver.cpp:1706:14 #7 0x25f5113 in CXFA_ImageRenderer::StartDIBSource() xfa/fxfa/app/xfa_ffwidget.cpp:626:18 #8 0x25f7986 in Start xfa/fxfa/app/xfa_ffwidget.cpp:622:10 #9 0x25f7986 in XFA_DrawImage(CFX_Graphics*, CFX_RTemplate<float> const&, CFX_Matrix*, CFX_DIBitmap*, int, int, int, int, int) xfa/fxfa/app/xfa_ffwidget.cpp:899 #10 0x27b1301 in CXFA_FFImage::RenderWidget(CFX_Graphics*, CFX_Matrix*, unsigned int) xfa/fxfa/app/xfa_ffimage.cpp:63:5 #11 0x2614dd2 in CXFA_RenderContext::DoRender(IFX_Pause*) xfa/fxfa/app/xfa_rendercontext.cpp:60:16 #12 0x1c4b41d in CPDFSDK_PageView::PageView_OnDraw(CFX_RenderDevice*, CFX_Matrix*, CPDF_RenderOptions*, FX_RECT const&) fpdfsdk/cpdfsdk_pageview.cpp:112:21 #13 0x1c343ec in FFLCommon fpdfsdk/fpdfformfill.cpp:132:16 #14 0x1c343ec in FPDF_FFLDraw fpdfsdk/fpdfformfill.cpp:407 #15 0x4f5da5 in RenderPage(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, void*, void*&, FPDF_FORMFILLINFO_PDFiumTest&, int, Options const&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&) samples/pdfium_test.cc:594:5 #16 0x4f735d in RenderPdf(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, char const*, unsigned long, Options const&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&) samples/pdfium_test.cc:786:9 #17 0x4f8878 in main samples/pdfium_test.cc:916:5 #18 0x7ffff69d382f in __libc_start_main /build/glibc-GKVZIf/glibc-2.23/csu/../csu/libc-start.c:291 SUMMARY: AddressSanitizer: heap-buffer-overflow core/fxge/dib/fx_dib_engine.cpp:245:36 in CWeightTable::Calc(int, int, int, int, int, int, int) Shadow bytes around the buggy address: 0x1000741dc090: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x1000741dc0a0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x1000741dc0b0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x1000741dc0c0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x1000741dc0d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 =>0x1000741dc0e0:[04]fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x1000741dc0f0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x1000741dc100: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x1000741dc110: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x1000741dc120: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x1000741dc130: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa Shadow byte legend (one shadow byte represents 8 application bytes): Addressable: 00 Partially addressable: 01 02 03 04 05 06 07 Heap left redzone: fa Heap right redzone: fb Freed heap region: fd Stack left redzone: f1 Stack mid redzone: f2 Stack right redzone: f3 Stack partial redzone: f4 Stack after return: f5 Stack use after scope: f8 Global redzone: f9 Global init order: f6 Poisoned by user: f7 Container overflow: fc Array cookie: ac Intra object redzone: bb ASan internal: fe Left alloca redzone: ca Right alloca redzone: cb ==21787==ABORTING
,
Oct 10 2016
,
Oct 11 2016
Passing off to thestig@ who has been reviewing the CL in comment #1.
,
Oct 17 2016
The following revision refers to this bug: https://pdfium.googlesource.com/pdfium.git/+/05923132ae08d45fbe957219775a48c55ee57aef commit 05923132ae08d45fbe957219775a48c55ee57aef Author: stackexploit <stackexploit@gmail.com> Date: Mon Oct 17 07:16:23 2016 Strengthen bounds check in CWeightTable::Calc * part II This CL implemented a better version of CWeightTable::GetPixelWeightSize(), which will calculate the size of array PixelWeight.m_Weights correctly to prevent potential heap buffer overflow conditions. BUG= chromium:654183 R=ochang@chromium.org, thestig@chromium.org, dsinclair@chromium.org Review-Url: https://codereview.chromium.org/2404453003 [modify] https://crrev.com/05923132ae08d45fbe957219775a48c55ee57aef/core/fxge/dib/fx_dib_engine.cpp
,
Oct 17 2016
,
Oct 17 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/665001ab4f668881d484579557d5e5f1ec22aa18 commit 665001ab4f668881d484579557d5e5f1ec22aa18 Author: pdfium-deps-roller <pdfium-deps-roller@chromium.org> Date: Mon Oct 17 08:39:34 2016 Roll src/third_party/pdfium/ 4dc112664..05923132a (1 commit). https://pdfium.googlesource.com/pdfium.git/+log/4dc112664e9d..05923132ae08 $ git log 4dc112664..05923132a --date=short --no-merges --format='%ad %ae %s' 2016-10-17 stackexploit Strengthen bounds check in CWeightTable::Calc * part II BUG= 654183 TBR=dsinclair@chromium.org Review-Url: https://codereview.chromium.org/2424743002 Cr-Commit-Position: refs/heads/master@{#425636} [modify] https://crrev.com/665001ab4f668881d484579557d5e5f1ec22aa18/DEPS
,
Oct 17 2016
,
Oct 18 2016
,
Oct 21 2016
,
Oct 21 2016
[Automated comment] DEPS changes referenced in bugdroid comments, needs manual review.
,
Oct 23 2016
+awhalley@ for M55 merge review
,
Oct 23 2016
,
Oct 24 2016
,
Oct 24 2016
Nice one, $3,500 for this report!
,
Oct 24 2016
,
Oct 24 2016
Looks good for M55.
,
Oct 24 2016
Approving merge to M55 branch 2883 based on comment #16. Please merge ASAP. Thank you.
,
Oct 24 2016
Will merge.
,
Oct 24 2016
The following revision refers to this bug: https://chrome-internal.googlesource.com/chrome/tools/buildspec/+/e84013dc2611fe399d435debdd36c6da9aab3664 commit e84013dc2611fe399d435debdd36c6da9aab3664 Author: Lei Zhang <thestig@google.com> Date: Mon Oct 24 20:54:36 2016
,
Oct 24 2016
Seems like M55 merge is done per comment #19. Is anything pending for M55? If not, please remove "Merge-Approved-55" label and apply "merge-merged-2883" label. Thank you.
,
Oct 24 2016
,
Oct 24 2016
,
Nov 29 2016
,
Nov 29 2016
Please credit to Ke Liu of Tencent's Xuanwu LAB for this issue, thanks.
,
Jan 4 2017
,
Jan 23 2017
This bug has been closed for more than 14 weeks. Removing security view restrictions. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Dec 12 2017
Sorry, but why such issue was rewarded ? (it’s just a question) Since the xfa feature isn’t enabled with google chrome and thus that Google chrome can’t open xfa pdf or PDF1.7?
,
Dec 14 2017
For #c27, please read #c1, hope it helps.
,
Apr 25 2018
|
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by stackexp...@gmail.com
, Oct 8 2016