php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login

go to bug id or search bugs for

Bug #79385 Global Out-of-Bounds Read
Submitted: 2020-03-15 14:44 UTC Modified: 2020-03-17 08:13 UTC
From: dev dot davidmatosse at outlook dot com Assigned: cmb (profile)
Status: Not a bug Package: GD related
PHP Version: Irrelevant OS: Linux
Private report: No CVE-ID: None
View Add Comment Developer Edit
Welcome! If you don't have a Git account, you can't do anything here.
You can add a comment by following this link or if you reported this bug, you can edit this bug over here.
(description)
Block user comment
Status: Assign to:
Package:
Bug Type:
Summary:
From: dev dot davidmatosse at outlook dot com
New email:
PHP Version: OS:

 

 [2020-03-15 14:44 UTC] dev dot davidmatosse at outlook dot com 
Description:
------------
While analysing the source code of libgd (located at github.com/libgd/libgd [1] and bundled 
version of php 7.4.3 [2]) was found that there is a global out-of-bounds read error in function dynamicGetbuf (gd_io_dp.c [3]), called by gdImageCreateFromWebpPtr or gdImageCreateFromJpegPtr 
functions.

Both arguments of presented functions (gdImageCreateFromWebpPtr(int size, void *data) and
gdImageCreateFromJpegPtr(int size, void *data)) are user supplied data with no prior sanitize. 

Whatever the call context in which we have size > that the length of the data, we trigger the
global out-of-bounds read.


> Study Case

1 - gdImageCreateFromWebpPtr [4] is a function used to load truecolor or create images from WebP
data already in memory. The function have two argumets, a size of the data in bytes and a pointer
to data (WebP data). 
In attempt to create the Webp image from data already in memory, we call the function gdImageCreateFromWebpPtr 
and pass to then 2 arguments (user controlled data), this information is used to create the gdIOCtx [5]
and we pass to gdImageCreateFromWebpCtx [6] to create the image. 

2 - on function gdImageCreateFromWebpCtx, in attempt to get the number of bytes read from gdIOCtx
we call gdGetBuf [7] which is resolved dynamically to dynamicGetbuf [8]

3 - at this point we have the following
	rlen     = size
	dp->data = data
    if we call the memcpy [9] with rlen set to length greater than the source (dp->data), it results
    in global out-of-bounds read.


> conclusion
After digging into the code a little further was found that to reach the vulnarable function [8],
we need a following call stack

@whatever_function@ --> gdGetBuf(dest, size, src) --> dynamicGetbuf(src_pkd, dest, size) --> memcpy(dest, src, size); [where 'size' and 'data' are user controllable] 

As example we have: gdImageCreateFromWebpPtr and gdImageCreateFromJpegPtr


> references
[1] - https://github.com/libgd/libgd
[2] - https://www.php.net/downloads.php#v7.4.3
[3] - https://github.com/libgd/libgd/blob/master/src/gd_io_dp.c
[4] - https://github.com/libgd/libgd/blob/master/src/gd_webp.c#L86
[5] - https://github.com/libgd/libgd/blob/master/src/gd_webp.c#L89
[6] - https://github.com/libgd/libgd/blob/master/src/gd_webp.c#L102
[7] - https://github.com/libgd/libgd/blob/master/src/gd_io.c#L211
[8] - https://github.com/libgd/libgd/blob/master/src/gd_io_dp.c#L270
[9] - https://github.com/libgd/libgd/blob/master/src/gd_io_dp.c#L302

Test script:
---------------
/*
 file:     poc.c
 to build: clang-9 poc.c -o poc -lgd -fsanitize=address
*/

#include <gd.h>

int main(){

	int size 	= 16;
	void *data 	= "AAAAAAAAAAAA";
	gdImageCreateFromWebpPtr (size, data);
	return 0;
}

Expected result:
----------------
Webp image generated.

Actual result:
--------------
==1611==ERROR: AddressSanitizer: global-buffer-overflow on address 0x0000004d4c1c at pc 0x000000431bb7 bp 0x7ffe7ebd0ef0 sp 0x7ffe7ebd06b0
READ of size 48 at 0x0000004d4c1c thread T0
    #0 0x431bb6 in memcpy (~/poc/poc+0x431bb6)
    #1 0x7f57a1f5a368 in dynamicGetbuf ~/libgd/src/gd_io_dp.c:302:2
    #2 0x7f57a1f59fb7 in gdGetBuf ~/libgd/src/gd_io.c:213:9
    #3 0x7f57a1f68dd2 in gdImageCreateFromWebpCtx ~/libgd/src/gd_webp.c:126:7
    #4 0x7f57a1f69061 in gdImageCreateFromWebpPtr ~/libgd/src/gd_webp.c:92:7
    #5 0x4c222f in main (~/poc/poc+0x4c222f)
    #6 0x7f57a103882f in __libc_start_main /build/glibc-LK5gWL/glibc-2.23/csu/../csu/libc-start.c:291
    #7 0x41ac48 in _start (~/poc/poc+0x41ac48)

0x0000004d4c1c is located 0 bytes to the right of global variable '<string literal>' defined in 'poc.c:10:16' (0x4d4c00) of size 28
  '<string literal>' is ascii string 'AAAAAAAAAAAAAAAAAAAAAAAAAAA'
SUMMARY: AddressSanitizer: global-buffer-overflow (~/poc/poc+0x431bb6) in memcpy
Shadow bytes around the buggy address:
  0x000080092930: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x000080092940: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x000080092950: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x000080092960: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x000080092970: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x000080092980: 00 00 00[04]f9 f9 f9 f9 00 00 00 00 00 00 00 00
  0x000080092990: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0000800929a0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0000800929b0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0000800929c0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0000800929d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07 
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
  Shadow gap:              cc
==1611==ABORTING

Patches

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2020-03-15 15:06 UTC] cmb@php.net
-Type: Bug +Type: Security -Private report: No +Private report: Yes
 [2020-03-15 15:06 UTC] cmb@php.net 
Tentatively assessing as security issue.
 [2020-03-16 00:03 UTC] stas@php.net 
I imagine this has to be reported to libgd maintainers? Especially given it reproduces without PHP in the picture at all? Or the problem is already fixed in libgd and PHP bundled one is behind?
 [2020-03-16 08:00 UTC] cmb@php.net
-Status: Open +Status: Not a bug -Type: Security +Type: Bug -Assigned To: +Assigned To: cmb
 [2020-03-16 08:00 UTC] cmb@php.net 
> Both arguments of presented functions
> (gdImageCreateFromWebpPtr(int size, void *data) and
> gdImageCreateFromJpegPtr(int size, void *data)) are user supplied
> data with no prior sanitize.

No, they are not.  Of course, the client of libgd is supposed to
pass valid values, just like when calling memcpy(), for instance.
 [2020-03-16 09:29 UTC] bugreports at gmail dot com 
> Of course, the client of libgd is supposed to pass valid values

that's not how security works in the real world
 [2020-03-16 11:22 UTC] cmb@php.net 
> that's not how security works in the real world

Harald, this is how C works; when passing (the address of) a
buffer to a function, the buffer's size has to be known by the
callee, and this is typically accomplished by passing the size of
the buffer as well.
 [2020-03-17 00:11 UTC] dev dot davidmatosse at outlook dot com 
> Harald, this is how C works; when passing (the address of) a
buffer to a function, the buffer's size has to be known by the
callee, 

This is indeed truth, let's look closer [1]

```
#define GD_WEBP_ALLOC_STEP (4*1024) // GD_WEBP_ALLOC_STEP = 4096
...
BGD_DECLARE(gdImagePtr) gdImageCreateFromWebpCtx (gdIOCtx * infile)
{
	...
 	uint8_t   *filedata = NULL;
	...
	unsigned char   *read, *temp;
	size_t size = 0, n;
        ...

	do {
		temp = gdRealloc(filedata, size+GD_WEBP_ALLOC_STEP); // -> realloc(filedata, 4096)
		if (temp) { // true
			filedata = temp;
			read = temp + size;
		} else {
                       ...
		}

		n = gdGetBuf(read, GD_WEBP_ALLOC_STEP, infile); 
                /*
                  int gdGetBuf(void *buf, int size, gdIOCtx *ctx)
                  {return (ctx->getBuf)(ctx, buf, size);}
                */
           ...
```
As we can see, on the first interaction gdGetBuf is called with size GD_WEBP_ALLOC_STEP

> and this is typically accomplished by passing the size of
the buffer as well.

but is not this size that is considered in memcpy call at dynamicGetbuf
<let's compile and debug this test code>
```
#include <gd.h>
/* clang-9 poc.c -o poc -lgd -fsanitize=address */
int main(){

	void *data  = "AAAAAAAAAAAAAAAAAAAAAAAAAAA";
	int  size   = 48;

	gdImageCreateFromWebpPtr (size, data);
	return 0;
}
``` 
debugging
```
gdb ./poc
$ b gdImageCreateFromWebpPtr
$ r
$ b dynamicGetbuf

$ c
Breakpoint 2, dynamicGetbuf (ctx=0x607000000090, buf=0x621000000100, len=4096) at gd_io_dp.c:276
# as we can see the size is 4096 (GD_WEBP_ALLOC_STEP)

$ disassemble dynamicGetbuf # here we break before the memcpy call

$ b *0x00007ffff7b8c361

$ p rlen
$1 = 48    # user defined size

$ x/4gx dp->data 
0x4d4c00 <.str>:	0x4141414141414141	0x4141414141414141
0x4d4c10 <.str+16>:	0x4141414141414141	0x0000000000414141
 # user defined data

$ p dp->pos
$3 = 0
```

as it sums up,
```
static int dynamicGetbuf(gdIOCtxPtr ctx, void *buf, int len)
{
        ... 
	memcpy(buf, (void *) ((char *)dp->data + dp->pos), rlen);
        ...
}
```
we call memcpy with large dest (buf = 4096), arbitrary size (rlen) and
arbitrary source (dp->data), if rlen is greater than size of the dp->data
we read up ends of the dp->data, if buf (read on gdImageCreateFromWebpCtx)
were printed out later, we would have a situation similar to heartbleed[2]. 

[1] - https://github.com/libgd/libgd/blob/master/src/gd_webp.c
[2] - heartbleed.com
 [2020-03-17 08:13 UTC] cmb@php.net 
> <let's compile and debug this test code>

Why?  That code is broken in the first place, since

  gdImagePtr gdImageCreateFromWebpPtr (int size, void *data);

data is the start address of the buffer, size is the size of the
buffer[1].  So a correct test program would be like

int main(){

	void *data  = "AAAAAAAAAAAAAAAAAAAAAAAAAAA";
	int  size   = strlen(data) + 1;

	gdImageCreateFromWebpPtr (size, data);
	return 0;
}

And even if that fails with PHP's bundled libgd, that wouldn't
matter, because PHP does never call gdImageCreateFromWebpPtr().
It might still be an upstream issue, but to verify this, you
would have to test against libgd master branch.

[1] <http://libgd.github.io/manuals/2.2.5/files/gd_webp-c.html#gdImageCreateFromWebpPtr>
 
PHP Copyright © 2001-2024 The PHP Group
All rights reserved. 		Last updated: Fri May 03 02:01:31 2024 UTC