1 From cf7a8182c2642c50f1cf90dddea9ce96a8bad2e8 Mon Sep 17 00:00:00 2001
2 From: =?UTF-8?q?J=C3=B6rn=20Heusipp?= <osmanx@problemloesungsmaschine.de>
3 Date: Wed, 14 Jun 2017 12:25:40 +0200
4 Subject: [PATCH] src/common.c: Fix heap buffer overflows when writing strings
5 in binheader
6
7 Fixes the following problems:
8 1. Case 's' only enlarges the buffer by 16 bytes instead of size bytes.
9 2. psf_binheader_writef() enlarges the header buffer (if needed) prior to the
10 big switch statement by an amount (16 bytes) which is enough for all cases
11 where only a single value gets added. Cases 's', 'S', 'p' however
12 additionally write an arbitrary length block of data and again enlarge the
13 buffer to the required amount. However, the required space calculation does
14 not take into account the size of the length field which gets output before
15 the data.
16 3. Buffer size requirement calculation in case 'S' does not account for the
17 padding byte ("size += (size & 1) ;" happens after the calculation which
18 uses "size").
19 4. Case 'S' can overrun the header buffer by 1 byte when no padding is
20 involved
21 ("memcpy (&(psf->header.ptr [psf->header.indx]), strptr, size + 1) ;" while
22 the buffer is only guaranteed to have "size" space available).
23 5. "psf->header.ptr [psf->header.indx] = 0 ;" in case 'S' always writes 1 byte
24 beyond the space which is guaranteed to be allocated in the header buffer.
25 6. Case 's' can overrun the provided source string by 1 byte if padding is
26 involved ("memcpy (&(psf->header.ptr [psf->header.indx]), strptr, size) ;"
27 where "size" is "strlen (strptr) + 1" (which includes the 0 terminator,
28 plus optionally another 1 which is padding and not guaranteed to be
29 readable via the source string pointer).
30
31 Closes: https://github.com/erikd/libsndfile/issues/292
32 ---
33 src/common.c | 15 +++++++--------
34 1 file changed, 7 insertions(+), 8 deletions(-)
35
36 --- a/src/common.c
37 +++ b/src/common.c
38 @@ -675,16 +675,16 @@ psf_binheader_writef (SF_PRIVATE *psf, c
39 /* Write a C string (guaranteed to have a zero terminator). */
40 strptr = va_arg (argptr, char *) ;
41 size = strlen (strptr) + 1 ;
42 - size += (size & 1) ;
43
44 - if (psf->header.indx + (sf_count_t) size >= psf->header.len && psf_bump_header_allocation (psf, 16))
45 + if (psf->header.indx + 4 + (sf_count_t) size + (sf_count_t) (size & 1) > psf->header.len && psf_bump_header_allocation (psf, 4 + size + (size & 1)))
46 return count ;
47
48 if (psf->rwf_endian == SF_ENDIAN_BIG)
49 - header_put_be_int (psf, size) ;
50 + header_put_be_int (psf, size + (size & 1)) ;
51 else
52 - header_put_le_int (psf, size) ;
53 + header_put_le_int (psf, size + (size & 1)) ;
54 memcpy (&(psf->header.ptr [psf->header.indx]), strptr, size) ;
55 + size += (size & 1) ;
56 psf->header.indx += size ;
57 psf->header.ptr [psf->header.indx - 1] = 0 ;
58 count += 4 + size ;
59 @@ -697,16 +697,15 @@ psf_binheader_writef (SF_PRIVATE *psf, c
60 */
61 strptr = va_arg (argptr, char *) ;
62 size = strlen (strptr) ;
63 - if (psf->header.indx + (sf_count_t) size > psf->header.len && psf_bump_header_allocation (psf, size))
64 + if (psf->header.indx + 4 + (sf_count_t) size + (sf_count_t) (size & 1) > psf->header.len && psf_bump_header_allocation (psf, 4 + size + (size & 1)))
65 return count ;
66 if (psf->rwf_endian == SF_ENDIAN_BIG)
67 header_put_be_int (psf, size) ;
68 else
69 header_put_le_int (psf, size) ;
70 - memcpy (&(psf->header.ptr [psf->header.indx]), strptr, size + 1) ;
71 + memcpy (&(psf->header.ptr [psf->header.indx]), strptr, size + (size & 1)) ;
72 size += (size & 1) ;
73 psf->header.indx += size ;
74 - psf->header.ptr [psf->header.indx] = 0 ;
75 count += 4 + size ;
76 break ;
77
78 @@ -718,7 +717,7 @@ psf_binheader_writef (SF_PRIVATE *psf, c
79 size = (size & 1) ? size : size + 1 ;
80 size = (size > 254) ? 254 : size ;
81
82 - if (psf->header.indx + (sf_count_t) size > psf->header.len && psf_bump_header_allocation (psf, size))
83 + if (psf->header.indx + 1 + (sf_count_t) size > psf->header.len && psf_bump_header_allocation (psf, 1 + size))
84 return count ;
85
86 header_put_byte (psf, size) ;
|