Bug 698073 - Potential null pointer dereference in mem_word_get_bits_rectangle(gdevmem.c)
Summary: Potential null pointer dereference in mem_word_get_bits_rectangle(gdevmem.c)
Status: RESOLVED FIXED
Alias: None
Product: Ghostscript
Classification: Unclassified
Component: Graphics Library (show other bugs)
Version: master
Hardware: PC Linux
: P4 normal
Assignee: Default assignee
URL:
Keywords:
: 698078 (view as bug list)
Depends on:
Blocks:
 
Reported: 2017-06-18 23:46 UTC by ruc.iser
Modified: 2017-06-20 07:16 UTC (History)
0 users

See Also:
Customer:
Word Size: ---


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description ruc.iser 2017-06-18 23:46:34 UTC
Using a technique based on similarity computing, we have found a function mem_word_get_bits_rectangle() in gdevmem.c which is similar to the function mem_get_bits_rectangle() in Bug #698066. So this function may result in null pointer dereference as well.

/base/gdevmem.c
int mem_word_get_bits_rectangle(gx_device * dev, const gs_int_rect * prect,
                       gs_get_bits_params_t * params, gs_int_rect ** unread)
{
    gx_device_memory * const mdev = (gx_device_memory *)dev;
    byte *src;
    uint dev_raster = gx_device_raster(dev, 1);
    int x = prect->p.x;
    int w = prect->q.x - x;
    int y = prect->p.y;
    int h = prect->q.y - y;
    int bit_x, bit_w;
    int code;

    fit_fill_xywh(dev, x, y, w, h);
    if (w <= 0 || h <= 0) {
        x = y = w = h = 0;
    }
    bit_x = x * dev->color_info.depth;
    bit_w = w * dev->color_info.depth;
    src = scan_line_base(mdev, y);     //may result in null pointer dereference
   ...
}

Advised Patch:
int mem_word_get_bits_rectangle(gx_device * dev, const gs_int_rect * prect,
                       gs_get_bits_params_t * params, gs_int_rect ** unread)
{
   ...
    bit_x = x * dev->color_info.depth;
    bit_w = w * dev->color_info.depth;
+   
+   if (mdev->line_ptrs == 0x00)
+       return_error(gs_error_rangecheck);
+
    src = scan_line_base(mdev, y);
   ...
}
Comment 1 Ken Sharp 2017-06-19 00:35:27 UTC
mem_word_get_bits_rectangle() is a 'wrapper' for get_bits_rectangle and simply gets the bits using words. Since get_bits_rectangle() checks line_ptrs there is no need for mem_word_get_bits_rectangle() to do so.
Comment 2 ruc.iser 2017-06-19 01:00:14 UTC
(In reply to Ken Sharp from comment #1)
> mem_word_get_bits_rectangle() is a 'wrapper' for get_bits_rectangle and
> simply gets the bits using words. Since get_bits_rectangle() checks
> line_ptrs there is no need for mem_word_get_bits_rectangle() to do so.

But in this 'wrapper' function, the access to line_ptrs is before calling get_bits_rectangle(). Can the checking in get_bits_rectangle() protect the dereference in the 'wrapper'?
Comment 3 Ken Sharp 2017-06-19 01:15:13 UTC
(In reply to ruc.iser from comment #2)

> But in this 'wrapper' function, the access to line_ptrs is before calling
> get_bits_rectangle(). Can the checking in get_bits_rectangle() protect the
> dereference in the 'wrapper'?

OK that's a fair point, but can you please put this kind of explanation of the problem in the report?

Not simply put a (C++) comment in the copied code, that's much too easy to miss.
Comment 4 ruc.iser 2017-06-19 01:40:04 UTC
(In reply to Ken Sharp from comment #3)
> (In reply to ruc.iser from comment #2)
> 
> > But in this 'wrapper' function, the access to line_ptrs is before calling
> > get_bits_rectangle(). Can the checking in get_bits_rectangle() protect the
> > dereference in the 'wrapper'?
> 
> OK that's a fair point, but can you please put this kind of explanation of
> the problem in the report?
> 
> Not simply put a (C++) comment in the copied code, that's much too easy to
> miss.

Ok. We did not make the report clear enough, it's out fault. We'll pay attention to that in the future.
Comment 5 ruc.iser 2017-06-19 17:46:24 UTC
(In reply to Ken Sharp from comment #3)
> (In reply to ruc.iser from comment #2)
> 
> > But in this 'wrapper' function, the access to line_ptrs is before calling
> > get_bits_rectangle(). Can the checking in get_bits_rectangle() protect the
> > dereference in the 'wrapper'?
> 
> OK that's a fair point, but can you please put this kind of explanation of
> the problem in the report?
> 
> Not simply put a (C++) comment in the copied code, that's much too easy to
> miss.

So should we resubmit a report for this problem to make the explanation clearer?
Comment 6 Ken Sharp 2017-06-20 00:24:45 UTC
(In reply to ruc.iser from comment #5)
> (In reply to Ken Sharp from comment #3)
> > (In reply to ruc.iser from comment #2)
> > 
> > > But in this 'wrapper' function, the access to line_ptrs is before calling
> > > get_bits_rectangle(). Can the checking in get_bits_rectangle() protect the
> > > dereference in the 'wrapper'?
> > 
> > OK that's a fair point, but can you please put this kind of explanation of
> > the problem in the report?
> > 
> > Not simply put a (C++) comment in the copied code, that's much too easy to
> > miss.
> 
> So should we resubmit a report for this problem to make the explanation
> clearer?

No, that's OK, I changed the status back to unconfirmed from resolved. I understand your point now. I have a patch ready to commit, just waiting on review.
Comment 7 Ken Sharp 2017-06-20 00:25:32 UTC
*** Bug 698078 has been marked as a duplicate of this bug. ***