Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

4 bugs found in svgpp #70

Closed
ghost opened this issue Jan 12, 2019 · 2 comments
Closed

4 bugs found in svgpp #70

ghost opened this issue Jan 12, 2019 · 2 comments

Comments

@ghost
Copy link

ghost commented Jan 12, 2019

bug1: an oob-read bug

description
A type confusion bug lead to out-of-bound read in src/demo/svgpp_agg_render. buffer.pixfmt() should return a reference to pixfmt_alpha_blend_rgba but instead return some struct that look like a long+string. That is why it crashes when it simply get the member stride. This bug may be used in info-leak.
poc
https://drive.google.com/open?id=1J1jMmXyUyDNfDnAG2I8u7DsFirFo2eSD
asan
https://drive.google.com/open?id=1ZRbEzqun8BkGdXsRAG6tKsjd7v2gWbnV
details
The crash happened in src/demo/svgpp_agg_render
https://github.com/svgpp/svgpp/blob/master/src/demo/render/svgpp_render.cpp#L1705
Looks like type confusion. buffer.pixfmt() should return a reference to pixfmt_alpha_blend_rgba but instead return some struct that look like a long+string. That is why it crashes when it simply get the member stride
This is a out of bound read bug.

bug2: a heap-buffer-overflow bug

description
An heap buffer overflow bug in svgpp_agg_render which may lead to code excution. In the render_scanlines_aa_solid function, the blend_hline function is called repeatedly multiple times. blend_hline is equivalent to the process of loop writing. Each call will write a piece of heap data, and multiple calls will overwrite the data in the heap.
poc
https://drive.google.com/open?id=1f4Eh4ozzTwQ3jqWAONuam0rl_YO5io6l
asan
https://drive.google.com/open?id=1l906xNqzpjNKxNkVMHRJsBHYQRSOIuFE
details
here is a heap overflow here, the specific function call process is as follows:
ClipBuffer::ClipBuffer(clip_buffer.cpp) ---------------->
render_scanlines_aa_solid(agg_renderer_scanline.h)------Multiple calls in a loop------> blend_hline(agg_renderer_base.h) ------------>
blend_hline(agg_pixfmt_gray.h)
In the render_scanlines_aa_solid function, the blend_hline function is called repeatedly multiple times.
blend_hline is equivalent to the process of loop writing. Each call will write a piece of heap data, and multiple calls will overwrite the data in the heap.

void blend_hline(int x, int y,

In the render_scanlines_aa_solid function, there is no explicit restriction on the 'for' loop. After calling blend_hline multiple times, it causes a heap overflow.
This bug is also found in agg_pixfmt_rgb.h, agg_pixfmt_rgb_packed.h, agg_pixfmt_rgba.h, agg_pixfmt_transposer.h.

bug3: a stack overflow bug

description
in the function agg::cell_aa::not_equal, dx is assigned to (x2 - x1). if dx >= dx_limit, which is (16384 << poly_subpixel_shift). This function will call itself recursively.
There will be a sitaution when (x2 - x1) always bigger than dx_limit during the recursion
Thats how the stack-overflow happened. This bug mat lead to code execution.
poc
https://drive.google.com/open?id=1Ejzmnrk6WhUMQT6O6qfRy0dGF82kgl5X
asan
https://drive.google.com/open?id=17gPhe2-zJLSOFyx4PmpWdx1jeRYcvK2s
details
in the function agg::cell_aa::not_equal

 template<class Cell> 
    void rasterizer_cells_aa<Cell>::line(int x1, int y1, int x2, int y2)
    {
        enum dx_limit_e { dx_limit = 16384 << poly_subpixel_shift };

        int dx = x2 - x1;

        if(dx >= dx_limit || dx <= -dx_limit)
        {
            int cx = (x1 + x2) >> 1;
            int cy = (y1 + y2) >> 1;
            line(x1, y1, cx, cy);
            line(cx, cy, x2, y2);
        }

int dy = y2 - y1;

dx is assigned to (x2 - x1)
if dx >= dx_limit, which is (16384 << poly_subpixel_shift). This function will call itself recursively.
There will be a sitaution when (x2 - x1) always bigger than dx_limit during the recursion
Thats how the stack-overflow happened.

bug4: a oob-read bug

description
After callng gil::get_color function in the boost library, the return code is used as an address.
Thus it caused an Violation Access. This may lead to an out-of-bound read.
poc
https://drive.google.com/open?id=1o1EqHQ6sjY68IRH24yokqYqgTMHSOqe6
asan
https://drive.google.com/open?id=1gYkV4ql71p6xN43aBa3JxBxLAwsprkEh
details

gil::get_color(result, gil::green_t()) 
      = gil_detail::blend_channel_fn<BlendModeTag, typename gil::color_element_type<Color, gil::green_t>::type>()(
        gil::get_color(pixa, gil::green_t()), gil::get_color(pixb, gil::green_t()),
alpha_a, alpha_b);

After callng gil::get_color function in the boost library, the return code is used as an address.
Thus it caused an Violation Access.

@svgpp
Copy link
Owner

svgpp commented Jan 13, 2019

Hi,
Thank you for a great job.
I've shipped fixes for bugs 1 and 4. Bug 3 is already fixed in AGG library. I'll schedule update for the latest AGG.

svgpp added a commit that referenced this issue Jan 14, 2019
@svgpp svgpp closed this as completed Jan 14, 2019
@nluedtke
Copy link

nluedtke commented Feb 4, 2019

These were assigned CVE-2019-6245, CVE-2019-6246, and CVE-2019-6247.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants