Skip to content

Fix buffer over-read on sbasis_to_cubic_bezier()

Yuki Hoshino requested to merge yhoshi/lib2geom:fix-746 into master

If (sb[X].size() == 1 && sb[Y].size() >= 3) or (sb[X].size() >= 3 && sb[Y].size() == 1), sbasis_to_cubic_bezier() accesses sb[X][1] and sb[Y][1]. It is buffer over-read and causes crush. If sb[].size() is not enough length, set 0 instead of sb[][1][].

Suppress crush for inkscape#746

Test code:

#include <iostream>
#include <vector>
#include "2geom/d2.h"
#include "2geom/linear.h"
#include "2geom/sbasis.h"
#include "2geom/sbasis-to-bezier.h"
#include "2geom/point.h"

int main(int argc, char *argb[]){
        Geom::D2<Geom::SBasis> sb;
        sb[Geom::X][0][0] = 0;
        sb[Geom::X][0][1] = 1;
        sb[Geom::Y] = Geom::SBasis(0, 0, 0, 0, 0, 1);
        std::cout << sb[Geom::X].size() << " " << sb[Geom::Y].size() << std::endl;
        std::vector<Geom::Point> bz;
        Geom::sbasis_to_cubic_bezier(bz, sb);
        for(const auto &point: bz){
                std::cout << point << std::endl;
        }
}

Output before this patch.

$ valgrind ./test
==481144== Memcheck, a memory error detector
==481144== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==481144== Using Valgrind-3.17.0 and LibVEX; rerun with -h for copyright info
==481144== Command: ./test
==481144==
1 3
==481144== Invalid read of size 8
==481144==    at 0x40D79D: operator[] (sbasis.h:80)
==481144==    by 0x40D79D: Geom::sbasis_to_cubic_bezier(std::vector<Geom::Point, std::allocator<Geom::Point> >&, Geom::D2<Geom::SBasis> const&) (sbasis-to-bezier.cpp:243)
==481144==    by 0x40A9B3: main (test2.cpp:16)
==481144==  Address 0x50fb0e0 is 0 bytes after a block of size 16 alloc'd
==481144==    at 0x4840FF5: operator new(unsigned long) (vg_replace_malloc.c:417)
==481144==    by 0x40C30F: __gnu_cxx::new_allocator<Geom::Linear>::allocate(unsigned long, void const*) (new_allocator.h:127)
==481144==    by 0x40BF55: std::allocator_traits<std::allocator<Geom::Linear> >::allocate(std::allocator<Geom::Linear>&, unsigned long) (alloc_traits.h:460)
==481144==    by 0x40B941: std::_Vector_base<Geom::Linear, std::allocator<Geom::Linear> >::_M_allocate(unsigned long) (stl_vector.h:346)
==481144==    by 0x40BFE8: std::_Vector_base<Geom::Linear, std::allocator<Geom::Linear> >::_M_create_storage(unsigned long) (stl_vector.h:361)
==481144==    by 0x40BA8A: std::_Vector_base<Geom::Linear, std::allocator<Geom::Linear> >::_Vector_base(unsigned long, std::allocator<Geom::Linear> const&) (stl_vector.h:305)
==481144==    by 0x40B0C0: std::vector<Geom::Linear, std::allocator<Geom::Linear> >::vector(unsigned long, Geom::Linear const&, std::allocator<Geom::Linear> const&) (stl_vector.h:524)
==481144==    by 0x40AC63: Geom::SBasis::SBasis() (sbasis.h:113)
==481144==    by 0x40B64E: Geom::D2<Geom::SBasis>::D2() (d2.h:64)
==481144==    by 0x40A85A: main (test2.cpp:10)
==481144==
==481144== Invalid read of size 8
==481144==    at 0x40D7E4: operator[] (sbasis.h:80)
==481144==    by 0x40D7E4: Geom::sbasis_to_cubic_bezier(std::vector<Geom::Point, std::allocator<Geom::Point> >&, Geom::D2<Geom::SBasis> const&) (sbasis-to-bezier.cpp:243)
==481144==    by 0x40A9B3: main (test2.cpp:16)
==481144==  Address 0x50fb0e8 is 8 bytes after a block of size 16 alloc'd
==481144==    at 0x4840FF5: operator new(unsigned long) (vg_replace_malloc.c:417)
==481144==    by 0x40C30F: __gnu_cxx::new_allocator<Geom::Linear>::allocate(unsigned long, void const*) (new_allocator.h:127)
==481144==    by 0x40BF55: std::allocator_traits<std::allocator<Geom::Linear> >::allocate(std::allocator<Geom::Linear>&, unsigned long) (alloc_traits.h:460)
==481144==    by 0x40B941: std::_Vector_base<Geom::Linear, std::allocator<Geom::Linear> >::_M_allocate(unsigned long) (stl_vector.h:346)
==481144==    by 0x40BFE8: std::_Vector_base<Geom::Linear, std::allocator<Geom::Linear> >::_M_create_storage(unsigned long) (stl_vector.h:361)
==481144==    by 0x40BA8A: std::_Vector_base<Geom::Linear, std::allocator<Geom::Linear> >::_Vector_base(unsigned long, std::allocator<Geom::Linear> const&) (stl_vector.h:305)
==481144==    by 0x40B0C0: std::vector<Geom::Linear, std::allocator<Geom::Linear> >::vector(unsigned long, Geom::Linear const&, std::allocator<Geom::Linear> const&) (stl_vector.h:524)
==481144==    by 0x40AC63: Geom::SBasis::SBasis() (sbasis.h:113)
==481144==    by 0x40B64E: Geom::D2<Geom::SBasis>::D2() (d2.h:64)
==481144==    by 0x40A85A: main (test2.cpp:10)
==481144==
(0, 0)
(0.3333333333333333, 0.3333333333333333)
(0.6666666666666667, 0.6666666666666667)
(1, 1)
==481144==
==481144== HEAP SUMMARY:
==481144==     in use at exit: 0 bytes in 0 blocks
==481144==   total heap usage: 28 allocs, 28 frees, 75,792 bytes allocated
==481144==
==481144== All heap blocks were freed -- no leaks are possible
==481144==
==481144== For lists of detected and suppressed errors, rerun with: -s
==481144== ERROR SUMMARY: 2 errors from 2 contexts (suppressed: 0 from 0)

Output after this patch:

$ valgrind ./test
==481412== Memcheck, a memory error detector
==481412== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==481412== Using Valgrind-3.17.0 and LibVEX; rerun with -h for copyright info
==481412== Command: ./test
==481412==
1 3
(0, 0)
(0.3333333333333333, 0.3333333333333333)
(0.6666666666666667, 0.6666666666666667)
(1, 1)
==481412==
==481412== HEAP SUMMARY:
==481412==     in use at exit: 0 bytes in 0 blocks
==481412==   total heap usage: 28 allocs, 28 frees, 75,792 bytes allocated
==481412==
==481412== All heap blocks were freed -- no leaks are possible
==481412==
==481412== For lists of detected and suppressed errors, rerun with: -s
==481412== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)

Merge request reports