blob: 491dae311489eea41fd2f44ab7f9466e06796db1 [file] [log] [blame]
Brad Bishop19323692019-04-05 15:28:33 -04001Heap-based Buffer Overflow in the psf_binheader_writef function in common.c in
2libsndfile through 1.0.28 allows remote attackers to cause a denial of service
3(application crash) or possibly have unspecified other impact.
4
5CVE: CVE-2017-12562
6Upstream-Status: Backport [cf7a8182c2642c50f1cf90dddea9ce96a8bad2e8]
7Signed-off-by: Ross Burton <ross.burton@intel.com>
8
9From b6a9d7e95888ffa77d8c75ce3f03e6c7165587cd Mon Sep 17 00:00:00 2001
10From: =?UTF-8?q?J=C3=B6rn=20Heusipp?= <osmanx@problemloesungsmaschine.de>
11Date: Wed, 14 Jun 2017 12:25:40 +0200
12Subject: [PATCH] src/common.c: Fix heap buffer overflows when writing strings
13 in binheader
14
15Fixes the following problems:
16 1. Case 's' only enlarges the buffer by 16 bytes instead of size bytes.
17 2. psf_binheader_writef() enlarges the header buffer (if needed) prior to the
18 big switch statement by an amount (16 bytes) which is enough for all cases
19 where only a single value gets added. Cases 's', 'S', 'p' however
20 additionally write an arbitrary length block of data and again enlarge the
21 buffer to the required amount. However, the required space calculation does
22 not take into account the size of the length field which gets output before
23 the data.
24 3. Buffer size requirement calculation in case 'S' does not account for the
25 padding byte ("size += (size & 1) ;" happens after the calculation which
26 uses "size").
27 4. Case 'S' can overrun the header buffer by 1 byte when no padding is
28 involved
29 ("memcpy (&(psf->header.ptr [psf->header.indx]), strptr, size + 1) ;" while
30 the buffer is only guaranteed to have "size" space available).
31 5. "psf->header.ptr [psf->header.indx] = 0 ;" in case 'S' always writes 1 byte
32 beyond the space which is guaranteed to be allocated in the header buffer.
33 6. Case 's' can overrun the provided source string by 1 byte if padding is
34 involved ("memcpy (&(psf->header.ptr [psf->header.indx]), strptr, size) ;"
35 where "size" is "strlen (strptr) + 1" (which includes the 0 terminator,
36 plus optionally another 1 which is padding and not guaranteed to be
37 readable via the source string pointer).
38
39Closes: https://github.com/erikd/libsndfile/issues/292
40---
41 src/common.c | 15 +++++++--------
42 1 file changed, 7 insertions(+), 8 deletions(-)
43
44diff --git a/src/common.c b/src/common.c
45index 1a6204ca..6b2a2ee9 100644
46--- a/src/common.c
47+++ b/src/common.c
48@@ -681,16 +681,16 @@ psf_binheader_writef (SF_PRIVATE *psf, const char *format, ...)
49 /* Write a C string (guaranteed to have a zero terminator). */
50 strptr = va_arg (argptr, char *) ;
51 size = strlen (strptr) + 1 ;
52- size += (size & 1) ;
53
54- if (psf->header.indx + (sf_count_t) size >= psf->header.len && psf_bump_header_allocation (psf, 16))
55+ 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)))
56 return count ;
57
58 if (psf->rwf_endian == SF_ENDIAN_BIG)
59- header_put_be_int (psf, size) ;
60+ header_put_be_int (psf, size + (size & 1)) ;
61 else
62- header_put_le_int (psf, size) ;
63+ header_put_le_int (psf, size + (size & 1)) ;
64 memcpy (&(psf->header.ptr [psf->header.indx]), strptr, size) ;
65+ size += (size & 1) ;
66 psf->header.indx += size ;
67 psf->header.ptr [psf->header.indx - 1] = 0 ;
68 count += 4 + size ;
69@@ -703,16 +703,15 @@ psf_binheader_writef (SF_PRIVATE *psf, const char *format, ...)
70 */
71 strptr = va_arg (argptr, char *) ;
72 size = strlen (strptr) ;
73- if (psf->header.indx + (sf_count_t) size > psf->header.len && psf_bump_header_allocation (psf, size))
74+ 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)))
75 return count ;
76 if (psf->rwf_endian == SF_ENDIAN_BIG)
77 header_put_be_int (psf, size) ;
78 else
79 header_put_le_int (psf, size) ;
80- memcpy (&(psf->header.ptr [psf->header.indx]), strptr, size + 1) ;
81+ memcpy (&(psf->header.ptr [psf->header.indx]), strptr, size + (size & 1)) ;
82 size += (size & 1) ;
83 psf->header.indx += size ;
84- psf->header.ptr [psf->header.indx] = 0 ;
85 count += 4 + size ;
86 break ;
87
88@@ -724,7 +723,7 @@ psf_binheader_writef (SF_PRIVATE *psf, const char *format, ...)
89 size = (size & 1) ? size : size + 1 ;
90 size = (size > 254) ? 254 : size ;
91
92- if (psf->header.indx + (sf_count_t) size > psf->header.len && psf_bump_header_allocation (psf, size))
93+ if (psf->header.indx + 1 + (sf_count_t) size > psf->header.len && psf_bump_header_allocation (psf, 1 + size))
94 return count ;
95
96 header_put_byte (psf, size) ;