Removed
Link Here
|
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 |
--- src/common.c.orig 2017-04-02 06:33:16 UTC |
33 |
+++ src/common.c |
34 |
@@ -681,16 +681,16 @@ psf_binheader_writef (SF_PRIVATE *psf, const char *format, ...) |
35 |
/* Write a C string (guaranteed to have a zero terminator). */ |
36 |
strptr = va_arg (argptr, char *) ; |
37 |
size = strlen (strptr) + 1 ; |
38 |
- size += (size & 1) ; |
39 |
|
40 |
- if (psf->header.indx + (sf_count_t) size >= psf->header.len && psf_bump_header_allocation (psf, 16)) |
41 |
+ 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))) |
42 |
return count ; |
43 |
|
44 |
if (psf->rwf_endian == SF_ENDIAN_BIG) |
45 |
- header_put_be_int (psf, size) ; |
46 |
+ header_put_be_int (psf, size + (size & 1)) ; |
47 |
else |
48 |
- header_put_le_int (psf, size) ; |
49 |
+ header_put_le_int (psf, size + (size & 1)) ; |
50 |
memcpy (&(psf->header.ptr [psf->header.indx]), strptr, size) ; |
51 |
+ size += (size & 1) ; |
52 |
psf->header.indx += size ; |
53 |
psf->header.ptr [psf->header.indx - 1] = 0 ; |
54 |
count += 4 + size ; |
55 |
@@ -703,16 +703,15 @@ psf_binheader_writef (SF_PRIVATE *psf, const char *format, ...) |
56 |
*/ |
57 |
strptr = va_arg (argptr, char *) ; |
58 |
size = strlen (strptr) ; |
59 |
- if (psf->header.indx + (sf_count_t) size > psf->header.len && psf_bump_header_allocation (psf, size)) |
60 |
+ 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))) |
61 |
return count ; |
62 |
if (psf->rwf_endian == SF_ENDIAN_BIG) |
63 |
header_put_be_int (psf, size) ; |
64 |
else |
65 |
header_put_le_int (psf, size) ; |
66 |
- memcpy (&(psf->header.ptr [psf->header.indx]), strptr, size + 1) ; |
67 |
+ memcpy (&(psf->header.ptr [psf->header.indx]), strptr, size + (size & 1)) ; |
68 |
size += (size & 1) ; |
69 |
psf->header.indx += size ; |
70 |
- psf->header.ptr [psf->header.indx] = 0 ; |
71 |
count += 4 + size ; |
72 |
break ; |
73 |
|
74 |
@@ -724,7 +723,7 @@ psf_binheader_writef (SF_PRIVATE *psf, const char *format, ...) |
75 |
size = (size & 1) ? size : size + 1 ; |
76 |
size = (size > 254) ? 254 : size ; |
77 |
|
78 |
- if (psf->header.indx + (sf_count_t) size > psf->header.len && psf_bump_header_allocation (psf, size)) |
79 |
+ if (psf->header.indx + 1 + (sf_count_t) size > psf->header.len && psf_bump_header_allocation (psf, 1 + size)) |
80 |
return count ; |
81 |
|
82 |
header_put_byte (psf, size) ; |