Undefined Behavior Case

[m-carrasco]

Hi 👋

I'd like to report the following test case that triggers UB in lunasvg.

The file can be found here. The harness is a slightly modified version of svg2png. It calls Document::loadFromData(buffer, size); instead of Document::loadFromFile(filename);. The test case comes from fuzzing.

An important remark I think is that this test case is not rejected by Document::loadFromData or bitmap.valid().

Build settings:

1. git clone -b test-cases https://github.com/m-carrasco/lunasvg.git
2. mkdir build && cd build
3. CXX=clang++-12 CC=clang-12 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" ../lunasvg/

How to execute:

1. UBSAN_OPTIONS=halt_on_error=1:print_stacktrace=1 ASAN_OPTIONS=halt_on_error=1 build/example/svg2png lunasvg/test-cases/e-feMorphology-009.bmfull_rand_base.ts3.e18.s0.sid451.smtbit_opt_uc.svg

Error log:

/lunasvg/3rdparty/plutovg/plutovg-blend.c:147:16: runtime error: -nan is outside the range of representable values of type 'int'
    #0 0x7a689f in gradient_pixel /lunasvg/3rdparty/plutovg/plutovg-blend.c:147:16
    #1 0x7a8948 in fetch_radial_gradient /lunasvg/3rdparty/plutovg/plutovg-blend.c:264:25
    #2 0x79a2b8 in blend_radial_gradient /lunasvg/3rdparty/plutovg/plutovg-blend.c:482:13
    #3 0x793c7a in plutovg_blend_gradient /lunasvg/3rdparty/plutovg/plutovg-blend.c:790:9
    #4 0x7903e5 in plutovg_blend /lunasvg/3rdparty/plutovg/plutovg-blend.c:701:9
    #5 0x768a21 in plutovg_fill_preserve /lunasvg/3rdparty/plutovg/plutovg.c:463:5
    #6 0x7683d4 in plutovg_fill /lunasvg/3rdparty/plutovg/plutovg.c:423:5
    #7 0x6d5957 in lunasvg::Canvas::fill(lunasvg::Path const&, lunasvg::Transform const&, lunasvg::WindRule, lunasvg::BlendMode, double) /lunasvg/source/canvas.cpp:100:5
    #8 0x69cf72 in lunasvg::FillData::fill(lunasvg::RenderState&, lunasvg::Path const&) const /lunasvg/source/layoutcontext.cpp:331:19
    #9 0x69f8b8 in lunasvg::LayoutShape::render(lunasvg::RenderState&) const /lunasvg/source/layoutcontext.cpp:408:18
    #10 0x691683 in lunasvg::LayoutContainer::renderChildren(lunasvg::RenderState&) const /lunasvg/source/layoutcontext.cpp:88:16
    #11 0x694e4e in lunasvg::LayoutSymbol::render(lunasvg::RenderState&) const /lunasvg/source/layoutcontext.cpp:159:5
    #12 0x5bb566 in lunasvg::Document::render(lunasvg::Bitmap, lunasvg::Matrix const&) const /lunasvg/source/lunasvg.cpp:343:11
    #13 0x5bcc85 in lunasvg::Document::renderToBitmap(unsigned int, unsigned int, unsigned int) const /lunasvg/source/lunasvg.cpp:368:5
    #14 0x5ad057 in main /lunasvg/example/svg2png.cpp:70:29
    #15 0x7f6c49fe1082 in __libc_start_main /build/glibc-SzIz7B/glibc-2.31/csu/../csu/libc-start.c:308:16
    #16 0x4e6d6d in _start (/lunasvg-build-ubsan-asan/example/svg2png+0x4e6d6d)

Thanks a lot for sharing this project!

Best regards,
Manuel

[m-carrasco]

Fixed error in building instructions. Now it points to the correct repository.

[m-carrasco]

Hi @sammycage 👋

How are you? I just wanted to know if I could provide any extra information to help fix this bug.

From my understanding so far, I think the problem is that it was not rejected either by Document::loadFromData or bitmap.valid().

Do you have any opinions or thoughts on the cause of this issue?

Best regards,
Manuel

[sammycage]

I just wanted to know if I could provide any extra information to help fix this bug.

I need this file e-feMorphology-009.bmfull_rand_base.ts3.e18.s0.sid451.smtbit_opt_uc.svg

[m-carrasco]

Hi @sammycage

Here is the file.

I should have made it available easier; sorry for that.

Best regards,
Manuel.

[sammycage]

Here is the file.

This file contains some invalid characters.

[sammycage]

There are no errors on my machine... Am I doing something wrong?

sammycage@ubuntu:~/Projects/lunasvg$ mkdir build && cd build
sammycage@ubuntu:~/Projects/lunasvg/build$ CXX=clang++-12 CC=clang-12 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" ..
-- The CXX compiler identification is Clang 12.0.1
-- The C compiler identification is Clang 12.0.1
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: /usr/bin/clang++-12 - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working C compiler: /usr/bin/clang-12 - skipped
-- Detecting C compile features
-- Detecting C compile features - done
-- Configuring done
-- Generating done
-- Build files have been written to: /home/sammycage/Projects/lunasvg/build
sammycage@ubuntu:~/Projects/lunasvg/build$ make -j3
[  3%] Building CXX object CMakeFiles/lunasvg.dir/source/lunasvg.cpp.o
[  9%] Building CXX object CMakeFiles/lunasvg.dir/source/property.cpp.o
[  9%] Building CXX object CMakeFiles/lunasvg.dir/source/element.cpp.o
[ 12%] Building CXX object CMakeFiles/lunasvg.dir/source/parser.cpp.o
[ 15%] Building CXX object CMakeFiles/lunasvg.dir/source/layoutcontext.cpp.o
[ 18%] Building CXX object CMakeFiles/lunasvg.dir/source/canvas.cpp.o
[ 21%] Building CXX object CMakeFiles/lunasvg.dir/source/clippathelement.cpp.o
[ 25%] Building CXX object CMakeFiles/lunasvg.dir/source/defselement.cpp.o
[ 28%] Building CXX object CMakeFiles/lunasvg.dir/source/gelement.cpp.o
[ 31%] Building CXX object CMakeFiles/lunasvg.dir/source/geometryelement.cpp.o
[ 34%] Building CXX object CMakeFiles/lunasvg.dir/source/graphicselement.cpp.o
[ 37%] Building CXX object CMakeFiles/lunasvg.dir/source/maskelement.cpp.o
[ 40%] Building CXX object CMakeFiles/lunasvg.dir/source/markerelement.cpp.o
[ 43%] Building CXX object CMakeFiles/lunasvg.dir/source/paintelement.cpp.o
[ 46%] Building CXX object CMakeFiles/lunasvg.dir/source/stopelement.cpp.o
[ 50%] Building CXX object CMakeFiles/lunasvg.dir/source/styledelement.cpp.o
[ 53%] Building CXX object CMakeFiles/lunasvg.dir/source/styleelement.cpp.o
[ 56%] Building CXX object CMakeFiles/lunasvg.dir/source/svgelement.cpp.o
[ 59%] Building CXX object CMakeFiles/lunasvg.dir/source/symbolelement.cpp.o
[ 62%] Building CXX object CMakeFiles/lunasvg.dir/source/useelement.cpp.o
[ 65%] Building C object CMakeFiles/lunasvg.dir/3rdparty/plutovg/plutovg.c.o
[ 68%] Building C object CMakeFiles/lunasvg.dir/3rdparty/plutovg/plutovg-paint.c.o
[ 71%] Building C object CMakeFiles/lunasvg.dir/3rdparty/plutovg/plutovg-geometry.c.o
[ 75%] Building C object CMakeFiles/lunasvg.dir/3rdparty/plutovg/plutovg-blend.c.o
[ 78%] Building C object CMakeFiles/lunasvg.dir/3rdparty/plutovg/plutovg-rle.c.o
[ 81%] Building C object CMakeFiles/lunasvg.dir/3rdparty/plutovg/plutovg-dash.c.o
[ 84%] Building C object CMakeFiles/lunasvg.dir/3rdparty/plutovg/plutovg-ft-raster.c.o
[ 87%] Building C object CMakeFiles/lunasvg.dir/3rdparty/plutovg/plutovg-ft-stroker.c.o
[ 90%] Building C object CMakeFiles/lunasvg.dir/3rdparty/plutovg/plutovg-ft-math.c.o
[ 93%] Linking CXX shared library liblunasvg.so
[ 93%] Built target lunasvg
[ 96%] Building CXX object example/CMakeFiles/svg2png.dir/svg2png.cpp.o
[100%] Linking CXX executable svg2png
[100%] Built target svg2png
sammycage@ubuntu:~/Projects/lunasvg/build$ UBSAN_OPTIONS=halt_on_error=1:print_stacktrace=1 ASAN_OPTIONS=halt_on_error=1 example/svg2png '/home/sammycage/Downloads/e-feMorphology-009.bmfull_rand_base.ts3.e18.s0.sid451.smtbit_opt_uc.svg'
Generated PNG file : e-feMorphology-009.bmfull_rand_base.ts3.e18.s0.sid451.smtbit_opt_uc.svg.png
sammycage@ubuntu:~/Projects/lunasvg/build$ 

[m-carrasco]

Hi @sammycage,

Thanks for checking this issue 😃

There are no errors on my machine... Am I doing something wrong?

To reproduce the bug, in svg2png, it is required to use Document::loadFromData(buffer, size); and load the entire test case content in buffer.

To do so, svg2png must look like this:

    // load all bytes into buffer
    std::uintmax_t size = std::filesystem::file_size(argv[1]);
    std::fstream f1;
    f1.open(argv[1], std::ios::in | std::ios::binary);
    char* buffer = (char*)calloc(size, sizeof(char));
    f1.read(buffer, size);
    f1.close();
    
    auto document = Document::loadFromData(buffer, size);
    free(buffer);

    if(!document) return help();

    auto bitmap = document->renderToBitmap(width, height, bgColor);

Here you have a patch to modify master as required:

diff --git a/example/CMakeLists.txt b/example/CMakeLists.txt
index 53ec5ef..25f4bd9 100644
--- a/example/CMakeLists.txt
+++ b/example/CMakeLists.txt
@@ -1,6 +1,6 @@
 cmake_minimum_required(VERSION 3.3)
 
-set(CMAKE_CXX_STANDARD 14)
+set(CMAKE_CXX_STANDARD 17)
 
 project(svg2png CXX)
 
diff --git a/example/svg2png.cpp b/example/svg2png.cpp
index f9c33d7..8f26807 100644
--- a/example/svg2png.cpp
+++ b/example/svg2png.cpp
@@ -3,6 +3,8 @@
 
 #include <iostream>
 #include <sstream>
+#include <filesystem>
+#include <fstream>
 
 #include <lunasvg.h>
 
@@ -48,7 +50,16 @@ int main(int argc, char** argv)
     std::uint32_t bgColor = 0x00000000;
     if(!setup(argc, argv, filename, width, height, bgColor)) return help();
 
-    auto document = Document::loadFromFile(filename);
+    std::uintmax_t size = std::filesystem::file_size(argv[1]);
+    std::fstream f1;
+    f1.open(argv[1], std::ios::in | std::ios::binary);
+    char* buffer = (char*)calloc(size, sizeof(char));
+    f1.read(buffer, size);
+    f1.close();
+
+    auto document = Document::loadFromData(buffer, size);
+    free(buffer);
+
     if(!document) return help();
 
     auto bitmap = document->renderToBitmap(width, height, bgColor);

Please note that how the bytes are loaded into buffer is orthogonal to the bug. The problem is that this file's content can cause UB if processed by Document::loadFromData, which is public. The above patch is equivalent to the changes in my branch, which I mentioned in the first comment on this issue.

To sum up, the steps are as follows:

1. Clone: git clone https://github.com/sammycage/lunasvg.git && cd lunasvg
2. Download the above patch: wget -O svg2png.diff https://gist.githubusercontent.com/m-carrasco/77bff1fb93f5bbee5e58843f5db90e6d/raw/881a55eca55139aabb7bc609c1c39e2d455000f9/.diff
3. Apply the patch: git apply svg2png.diff
4. Build: mkdir build && cd build && CXX=clang++-12 CC=clang-12 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
5. Download e-feMorphology-009.bmfull_rand_base.ts3.e18.s0.sid451.smtbit_opt_uc.svg: wget -O e-feMorphology-009.bmfull_rand_base.ts3.e18.s0.sid451.smtbit_opt_uc.svg https://github.com/m-carrasco/lunasvg/raw/test-cases/test-cases/e-feMorphology-009.bmfull_rand_base.ts3.e18.s0.sid451.smtbit_opt_uc.svg
6. Test: UBSAN_OPTIONS=halt_on_error=1:print_stacktrace=1 ASAN_OPTIONS=halt_on_error=1 ./example/svg2png e-feMorphology-009.bmfull_rand_base.ts3.e18.s0.sid451.smtbit_opt_uc.svg

I apologise for any confusion. Please let me know if I can help with anything else.

Best regards,
Manuel.

[m-carrasco]

Hi @sammycage

I minimized the SVG file I originally reported. This new SVG file is valid, shorter, and triggers the same UB error. You can use svg2png as it is in master to trigger the UB error. The steps to reproduce the bug are as follows:

1. Clone: git clone https://github.com/sammycage/lunasvg.git && cd lunasvg
2. Build with sanitisers: mkdir build && cd build && CXX=clang++ CC=clang 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 minimized svg: wget -O example.svg https://user-images.githubusercontent.com/3461126/225024410-286b1073-7d19-41b9-8c26-7641abe72579.svg (it is attached in this post, in case this fails)
4. Run with sanitisers: UBSAN_OPTIONS=halt_on_error=1:print_stacktrace=1 ASAN_OPTIONS=halt_on_error=1 ./example/svg2png example.svg

Minimized SVG file: example.svg

I hope this test case is more helpful in solving the issue. Let me know if I can still improve this report, and thanks for sharing this project.

Best regards,
Manuel

[sammycage]

Could you please try to reproduce this error with gcc? I'm encountering difficulties with clang.

[m-carrasco]

Hi @sammycage

Thanks for your quick response!

GCC's sanitiser does not detect the problem, but there is a real issue, as reported by clang: -nan is outside the range of representable values of type 'int'. This kind of problem could cause portability issues or non-deterministic behaviour when optimizations are enabled.

The reported line by clang is: int ipos = (int)(pos * (COLOR_TABLE_SIZE - 1) + 0.5); in the gradient_pixel function.

I checked isnan(pos * (COLOR_TABLE_SIZE - 1) + 0.5) right before the cast, and both compilers confirm that the casted value is a nan.

In addition, I checked the raised floating-point exceptions using fetestexcept(FE_ALL_EXCEPT), right before the cast. In both cases, it reports that FE_INVALID has been raised. This can be caused by a domain error when using a floating-point function.

The nan and the exception are caused when gradient_pixel is called: *buffer++ = gradient_pixel(gradient, sqrt(det) - b).

Using both compilers, I confirmed that det is negative zero, thus causing sqrt to return a nan and raise the exception.

To summarize, the sqrt call on a negative value (det) returns a nan. This nan flows into gradient_pixel and causes the UB reported by clang when casted to an integer.

---

I found two possible ways to fix the UB and still correctly render the SVG.

At another callsite of gradient_pixel, det signedness is checked before calling sqrt:

            if(det >= 0)
            {
                double w = sqrt(det) - b;
                if(gradient->radial.fr + v->dr * w >= 0)
                    result = gradient_pixel(gradient, w);
            }

I tested this change, which applies the same check, and it seems to work:

diff --git a/3rdparty/plutovg/plutovg-blend.c b/3rdparty/plutovg/plutovg-blend.c
index 6cbe76e..ec3925c 100644
--- a/3rdparty/plutovg/plutovg-blend.c
+++ b/3rdparty/plutovg/plutovg-blend.c
@@ -261,7 +261,9 @@ static void fetch_radial_gradient(uint32_t* buffer, const radial_gradient_values
     {
         while(buffer < end)
         {
-            *buffer++ = gradient_pixel(gradient, sqrt(det) - b);
+            if (det >= 0)
+                *buffer = gradient_pixel(gradient, sqrt(det) - b);
+            buffer++;
             det += delta_det;
             delta_det += delta_delta_det;
             b += delta_b;

Alternatively, I tested *buffer++ = gradient_pixel(gradient, sqrt(fabs(det)) - b) and it also seems to work. The fix computes the absolute value of det, which can be negative zero sometimes, before calling sqrt.

diff --git a/3rdparty/plutovg/plutovg-blend.c b/3rdparty/plutovg/plutovg-blend.c
index 6cbe76e..ea08d40 100644
--- a/3rdparty/plutovg/plutovg-blend.c
+++ b/3rdparty/plutovg/plutovg-blend.c
@@ -261,7 +261,7 @@ static void fetch_radial_gradient(uint32_t* buffer, const radial_gradient_values
     {
         while(buffer < end)
         {
-            *buffer++ = gradient_pixel(gradient, sqrt(det) - b);
+            *buffer++ = gradient_pixel(gradient, sqrt(fabs(det)) - b);
             det += delta_det;
             delta_det += delta_delta_det;
             b += delta_b;

Let me know if there is anything else I can do. I can also open a pull request if any of the fixes look good to you.

Best regards,
Manuel.

[sammycage]

Let me know if there is anything else I can do. I can also open a pull request if any of the fixes look good to you.

        while(buffer < end)
        {
            uint32_t result = 0; // .
            if(det >= 0)
                result = gradient_pixel(gradient, sqrt(det) - b);
            *buffer++ = result;
            det += delta_det;
            delta_det += delta_delta_det;
            b += delta_b;
        }

Just being curious, what are you using this library for?

[m-carrasco]

@sammycage I appreciate your quick response. I opened a PR for this fix.

Just being curious, what are you using this library for?

We are using it as a benchmark for a fuzzing research project. Thanks for sharing LunaSVG o/

[sammycage]

Thanks for your findings, this especially #119

[m-carrasco]

You're welcome. Thanks for your time as well and for sharing this project!