sammycage / lunasvg

lunasvg is a standalone SVG rendering library in C++
MIT License
818 stars 115 forks source link

Fix for UB caused by wrong call to memcpy. #125

Closed m-carrasco closed 1 year ago

m-carrasco commented 1 year ago

Hi :wave:

Issue

LunaSVG triggers undefined behavior violating memcpy(dst, src, count) specification. In particular, it calls memcpy on NULL pointers, despite count being equal to zero. The specification states that If either dest or src is an invalid or null pointer, the behavior is undefined, even if count is zero.. This violation can be triggered for src and dst. The UB is triggered in this line. GCC's sanitiser confirms this on valid SVG files.

lunasvg/3rdparty/plutovg/plutovg-ft-stroker.c:633:9: runtime error: null pointer passed as argument 2, which is declared to never be null
    #0 0x55ca45d8ac53 in ft_stroke_border_export lunasvg/3rdparty/plutovg/plutovg-ft-stroker.c:633
    #1 0x55ca45d99164 in PVG_FT_Stroker_ExportBorder lunasvg/3rdparty/plutovg/plutovg-ft-stroker.c:1750
    #2 0x55ca45d99190 in PVG_FT_Stroker_Export lunasvg/3rdparty/plutovg/plutovg-ft-stroker.c:1758
    #3 0x55ca45d67cbb in plutovg_rle_rasterize lunasvg/3rdparty/plutovg/plutovg-rle.c:239
    #4 0x55ca45d3f5b9 in plutovg_stroke_preserve lunasvg/3rdparty/plutovg/plutovg.c:470
    #5 0x55ca45d3e638 in plutovg_stroke lunasvg/3rdparty/plutovg/plutovg.c:429
    #6 0x55ca45cdf419 in lunasvg::Canvas::stroke(lunasvg::Path const&, lunasvg::Transform const&, double, lunasvg::LineCap, lunasvg::LineJoin, double, lunasvg::DashData const&, lunasvg::BlendMode, double) lunasvg/source/canvas.cpp:116
    #7 0x55ca45cbbd17 in lunasvg::StrokeData::stroke(lunasvg::RenderState&, lunasvg::Path const&) const lunasvg/source/layoutcontext.cpp:344
    #8 0x55ca45cbdf9d in lunasvg::LayoutShape::render(lunasvg::RenderState&) const lunasvg/source/layoutcontext.cpp:409
    #9 0x55ca45cb095b in lunasvg::LayoutContainer::renderChildren(lunasvg::RenderState&) const lunasvg/source/layoutcontext.cpp:88
    #10 0x55ca45cb4eb7 in lunasvg::LayoutSymbol::render(lunasvg::RenderState&) const lunasvg/source/layoutcontext.cpp:159
    #11 0x55ca45c33455 in lunasvg::Document::render(lunasvg::Bitmap, lunasvg::Matrix const&) const lunasvg/source/lunasvg.cpp:343
    #12 0x55ca45c34617 in lunasvg::Document::renderToBitmap(unsigned int, unsigned int, unsigned int) const lunasvg/source/lunasvg.cpp:368
    #13 0x55ca45c2cab3 in main lunasvg/example/svg2png.cpp:54
    #14 0x7f7137d1c082 in __libc_start_main ../csu/libc-start.c:308
    #15 0x55ca45c232bd in _start (lunasvg/build-gcc/example/svg2png+0x1e42bd)

UB can cause portability and security issues. In this PR, I submit a test case and a possible fix for this issue.

Test Case

example.svg

Steps

  1. Clone: git clone https://github.com/sammycage/lunasvg.git && cd lunasvg
  2. Build with sanitisers: mkdir build && cd build && cmake -DLUNASVG_BUILD_EXAMPLES=ON -DCMAKE_CXX_FLAGS="-g -O0 -fsanitize=undefined,address -fno-sanitize-recover=all" -DCMAKE_C_FLAGS="-g -O0 -fsanitize=undefined,address -fno-sanitize-recover=all" ../ && make -j4
  3. Download svg: wget -O example.svg https://user-images.githubusercontent.com/3461126/225971525-0b05594d-9492-4e79-a9f4-26ff2420acb0.svg
  4. Run with sanitisers: UBSAN_OPTIONS=halt_on_error=1:print_stacktrace=1 ASAN_OPTIONS=halt_on_error=1 ./example/svg2png example.svg

Fix

Check if both argument pointers are not NULL before calling memcpy. This still allows generating the correct png when calling svg2png.