Patrick Williams | ac13d5f | 2023-11-24 18:59:46 -0600 | [diff] [blame] | 1 | From c7ce5b0ebeeb58934825077d1324960aa0747718 Mon Sep 17 00:00:00 2001 |
| 2 | From: Alex Stewart <alex.stewart@ni.com> |
| 3 | Date: Tue, 10 Oct 2023 16:10:34 -0400 |
| 4 | Subject: [PATCH] mat4/mat5: fix int overflow in dataend calculation |
| 5 | |
| 6 | The clang sanitizer warns of a possible signed integer overflow when |
| 7 | calculating the `dataend` value in `mat4_read_header()`. |
| 8 | |
| 9 | ``` |
| 10 | src/mat4.c:323:41: runtime error: signed integer overflow: 205 * -100663296 cannot be represented in type 'int' |
| 11 | SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior src/mat4.c:323:41 in |
| 12 | src/mat4.c:323:48: runtime error: signed integer overflow: 838860800 * 4 cannot be represented in type 'int' |
| 13 | SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior src/mat4.c:323:48 in |
| 14 | ``` |
| 15 | |
| 16 | Cast the offending `rows` and `cols` ints to `sf_count_t` (the type of |
| 17 | `dataend` before performing the calculation, to avoid the issue. |
| 18 | |
| 19 | CVE: CVE-2022-33065 |
| 20 | Fixes: https://github.com/libsndfile/libsndfile/issues/789 |
| 21 | Fixes: https://github.com/libsndfile/libsndfile/issues/833 |
| 22 | |
| 23 | Upstream-Status: Backport [9a829113c88a51e57c1e46473e90609e4b7df151] |
| 24 | |
| 25 | Signed-off-by: Alex Stewart <alex.stewart@ni.com> |
| 26 | --- |
| 27 | src/mat4.c | 2 +- |
| 28 | 1 file changed, 1 insertion(+), 1 deletion(-) |
| 29 | |
| 30 | diff --git a/src/mat4.c b/src/mat4.c |
| 31 | index 0b1b414b..575683ba 100644 |
| 32 | --- a/src/mat4.c |
| 33 | +++ b/src/mat4.c |
| 34 | @@ -320,7 +320,7 @@ mat4_read_header (SF_PRIVATE *psf) |
| 35 | psf->filelength - psf->dataoffset, psf->sf.channels * psf->sf.frames * psf->bytewidth) ; |
| 36 | } |
| 37 | else if ((psf->filelength - psf->dataoffset) > psf->sf.channels * psf->sf.frames * psf->bytewidth) |
| 38 | - psf->dataend = psf->dataoffset + rows * cols * psf->bytewidth ; |
| 39 | + psf->dataend = psf->dataoffset + (sf_count_t) rows * (sf_count_t) cols * psf->bytewidth ; |
| 40 | |
| 41 | psf->datalength = psf->filelength - psf->dataoffset - psf->dataend ; |
| 42 | |
| 43 | From 842303f984b2081481e74cb84a9a24ecbe3dec1a Mon Sep 17 00:00:00 2001 |
| 44 | From: Alex Stewart <alex.stewart@ni.com> |
| 45 | Date: Wed, 11 Oct 2023 16:36:02 -0400 |
| 46 | Subject: [PATCH] au: avoid int overflow while calculating data_end |
| 47 | |
| 48 | At several points in au_read_header(), we calculate the functional end |
| 49 | of the data segment by adding the (int)au_fmt.dataoffset and the |
| 50 | (int)au_fmt.datasize. This can overflow the implicit int_32 return value |
| 51 | and cause undefined behavior. |
| 52 | |
| 53 | Instead, precalculate the value and assign it to a 64-bit |
| 54 | (sf_count_t)data_end variable. |
| 55 | |
| 56 | CVE: CVE-2022-33065 |
| 57 | Fixes: https://github.com/libsndfile/libsndfile/issues/833 |
| 58 | |
| 59 | Signed-off-by: Alex Stewart <alex.stewart@ni.com> |
| 60 | --- |
| 61 | src/au.c | 10 ++++++---- |
| 62 | 1 file changed, 6 insertions(+), 4 deletions(-) |
| 63 | |
| 64 | diff --git a/src/au.c b/src/au.c |
| 65 | index 62bd691d..f68f2587 100644 |
| 66 | --- a/src/au.c |
| 67 | +++ b/src/au.c |
| 68 | @@ -291,6 +291,7 @@ static int |
| 69 | au_read_header (SF_PRIVATE *psf) |
| 70 | { AU_FMT au_fmt ; |
| 71 | int marker, dword ; |
| 72 | + sf_count_t data_end ; |
| 73 | |
| 74 | memset (&au_fmt, 0, sizeof (au_fmt)) ; |
| 75 | psf_binheader_readf (psf, "pm", 0, &marker) ; |
| 76 | @@ -317,14 +318,15 @@ au_read_header (SF_PRIVATE *psf) |
| 77 | return SFE_AU_EMBED_BAD_LEN ; |
| 78 | } ; |
| 79 | |
| 80 | + data_end = (sf_count_t) au_fmt.dataoffset + (sf_count_t) au_fmt.datasize ; |
| 81 | if (psf->fileoffset > 0) |
| 82 | - { psf->filelength = au_fmt.dataoffset + au_fmt.datasize ; |
| 83 | + { psf->filelength = data_end ; |
| 84 | psf_log_printf (psf, " Data Size : %d\n", au_fmt.datasize) ; |
| 85 | } |
| 86 | - else if (au_fmt.datasize == -1 || au_fmt.dataoffset + au_fmt.datasize == psf->filelength) |
| 87 | + else if (au_fmt.datasize == -1 || data_end == psf->filelength) |
| 88 | psf_log_printf (psf, " Data Size : %d\n", au_fmt.datasize) ; |
| 89 | - else if (au_fmt.dataoffset + au_fmt.datasize < psf->filelength) |
| 90 | - { psf->filelength = au_fmt.dataoffset + au_fmt.datasize ; |
| 91 | + else if (data_end < psf->filelength) |
| 92 | + { psf->filelength = data_end ; |
| 93 | psf_log_printf (psf, " Data Size : %d\n", au_fmt.datasize) ; |
| 94 | } |
| 95 | else |
| 96 | From 0754d3380a54e3fbdde0f684b88955c80c79f58f Mon Sep 17 00:00:00 2001 |
| 97 | From: Alex Stewart <alex.stewart@ni.com> |
| 98 | Date: Wed, 11 Oct 2023 16:46:29 -0400 |
| 99 | Subject: [PATCH] avr: fix int overflow in avr_read_header() |
| 100 | |
| 101 | Pre-cast hdr.frames to sf_count_t, to provide the calculation with |
| 102 | enough numeric space to avoid an int-overflow. |
| 103 | |
| 104 | CVE: CVE-2022-33065 |
| 105 | Fixes: https://github.com/libsndfile/libsndfile/issues/833 |
| 106 | |
| 107 | Signed-off-by: Alex Stewart <alex.stewart@ni.com> |
| 108 | --- |
| 109 | src/avr.c | 2 +- |
| 110 | 1 file changed, 1 insertion(+), 1 deletion(-) |
| 111 | |
| 112 | diff --git a/src/avr.c b/src/avr.c |
| 113 | index 6c78ff69..1bc1ffc9 100644 |
| 114 | --- a/src/avr.c |
| 115 | +++ b/src/avr.c |
| 116 | @@ -162,7 +162,7 @@ avr_read_header (SF_PRIVATE *psf) |
| 117 | psf->endian = SF_ENDIAN_BIG ; |
| 118 | |
| 119 | psf->dataoffset = AVR_HDR_SIZE ; |
| 120 | - psf->datalength = hdr.frames * (hdr.rez / 8) ; |
| 121 | + psf->datalength = (sf_count_t) hdr.frames * (hdr.rez / 8) ; |
| 122 | |
| 123 | if (psf->fileoffset > 0) |
| 124 | psf->filelength = AVR_HDR_SIZE + psf->datalength ; |
| 125 | From 6ac31a68a614e2bba4a05b54e5558d6270c98376 Mon Sep 17 00:00:00 2001 |
| 126 | From: Alex Stewart <alex.stewart@ni.com> |
| 127 | Date: Wed, 11 Oct 2023 16:54:21 -0400 |
| 128 | Subject: [PATCH] sds: fix int overflow warning in sample calculations |
| 129 | |
| 130 | The sds_*byte_read() functions compose their uint_32 sample buffers by |
| 131 | shifting 7bit samples into a 32bit wide buffer, and adding them |
| 132 | together. Because the 7bit samples are stored in 32bit ints, code |
| 133 | fuzzers become concerned that the addition operation can overflow and |
| 134 | cause undefined behavior. |
| 135 | |
| 136 | Instead, bitwise-OR the bytes together - which should accomplish the |
| 137 | same arithmetic operation, without risking an int-overflow. |
| 138 | |
| 139 | CVE: CVE-2022-33065 |
| 140 | Fixes: https://github.com/libsndfile/libsndfile/issues/833 |
| 141 | |
| 142 | Signed-off-by: Alex Stewart <alex.stewart@ni.com> |
| 143 | |
| 144 | Do the same for the 3byte and 4byte read functions. |
| 145 | --- |
| 146 | src/sds.c | 6 +++--- |
| 147 | 1 file changed, 3 insertions(+), 3 deletions(-) |
| 148 | |
| 149 | diff --git a/src/sds.c b/src/sds.c |
| 150 | index 6bc76171..2a0f164c 100644 |
| 151 | --- a/src/sds.c |
| 152 | +++ b/src/sds.c |
| 153 | @@ -454,7 +454,7 @@ sds_2byte_read (SF_PRIVATE *psf, SDS_PRIVATE *psds) |
| 154 | |
| 155 | ucptr = psds->read_data + 5 ; |
| 156 | for (k = 0 ; k < 120 ; k += 2) |
| 157 | - { sample = arith_shift_left (ucptr [k], 25) + arith_shift_left (ucptr [k + 1], 18) ; |
| 158 | + { sample = arith_shift_left (ucptr [k], 25) | arith_shift_left (ucptr [k + 1], 18) ; |
| 159 | psds->read_samples [k / 2] = (int) (sample - 0x80000000) ; |
| 160 | } ; |
| 161 | |
| 162 | @@ -498,7 +498,7 @@ sds_3byte_read (SF_PRIVATE *psf, SDS_PRIVATE *psds) |
| 163 | |
| 164 | ucptr = psds->read_data + 5 ; |
| 165 | for (k = 0 ; k < 120 ; k += 3) |
| 166 | - { sample = (((uint32_t) ucptr [k]) << 25) + (ucptr [k + 1] << 18) + (ucptr [k + 2] << 11) ; |
| 167 | + { sample = (((uint32_t) ucptr [k]) << 25) | (ucptr [k + 1] << 18) | (ucptr [k + 2] << 11) ; |
| 168 | psds->read_samples [k / 3] = (int) (sample - 0x80000000) ; |
| 169 | } ; |
| 170 | |
| 171 | @@ -542,7 +542,7 @@ sds_4byte_read (SF_PRIVATE *psf, SDS_PRIVATE *psds) |
| 172 | |
| 173 | ucptr = psds->read_data + 5 ; |
| 174 | for (k = 0 ; k < 120 ; k += 4) |
| 175 | - { sample = (((uint32_t) ucptr [k]) << 25) + (ucptr [k + 1] << 18) + (ucptr [k + 2] << 11) + (ucptr [k + 3] << 4) ; |
| 176 | + { sample = (((uint32_t) ucptr [k]) << 25) | (ucptr [k + 1] << 18) | (ucptr [k + 2] << 11) | (ucptr [k + 3] << 4) ; |
| 177 | psds->read_samples [k / 4] = (int) (sample - 0x80000000) ; |
| 178 | } ; |
| 179 | |
| 180 | From 96428e1dd4998f1cd47df24f8fe9b0da35d7b947 Mon Sep 17 00:00:00 2001 |
| 181 | From: Alex Stewart <alex.stewart@ni.com> |
| 182 | Date: Wed, 11 Oct 2023 17:26:51 -0400 |
| 183 | Subject: [PATCH] aiff: fix int overflow when counting header elements |
| 184 | |
| 185 | aiff_read_basc_chunk() tries to count the AIFF header size by keeping |
| 186 | track of the bytes returned by psf_binheader_readf(). Though improbable, |
| 187 | it is technically possible for these added bytes to exceed the int-sized |
| 188 | `count` accumulator. |
| 189 | |
| 190 | Use a 64-bit sf_count_t type for `count`, to ensure that it always has |
| 191 | enough numeric space. |
| 192 | |
| 193 | CVE: CVE-2022-33065 |
| 194 | Fixes: https://github.com/libsndfile/libsndfile/issues/833 |
| 195 | |
| 196 | Signed-off-by: Alex Stewart <alex.stewart@ni.com> |
| 197 | --- |
| 198 | src/aiff.c | 2 +- |
| 199 | 1 file changed, 1 insertion(+), 1 deletion(-) |
| 200 | |
| 201 | diff --git a/src/aiff.c b/src/aiff.c |
| 202 | index a2bda8f4..6b244302 100644 |
| 203 | --- a/src/aiff.c |
| 204 | +++ b/src/aiff.c |
| 205 | @@ -1702,7 +1702,7 @@ static int |
| 206 | aiff_read_basc_chunk (SF_PRIVATE * psf, int datasize) |
| 207 | { const char * type_str ; |
| 208 | basc_CHUNK bc ; |
| 209 | - int count ; |
| 210 | + sf_count_t count ; |
| 211 | |
| 212 | count = psf_binheader_readf (psf, "E442", &bc.version, &bc.numBeats, &bc.rootNote) ; |
| 213 | count += psf_binheader_readf (psf, "E222", &bc.scaleType, &bc.sigNumerator, &bc.sigDenominator) ; |
| 214 | From b352c350d35bf978e4d3a32e5d9df1f2284445f4 Mon Sep 17 00:00:00 2001 |
| 215 | From: Alex Stewart <alex.stewart@ni.com> |
| 216 | Date: Wed, 11 Oct 2023 17:43:02 -0400 |
| 217 | Subject: [PATCH] ircam: fix int overflow in ircam_read_header() |
| 218 | |
| 219 | When reading the IRCAM header, it is possible for the calculated |
| 220 | blockwidth to exceed the bounds of a signed int32. |
| 221 | |
| 222 | Use a 64bit sf_count_t to store the blockwidth. |
| 223 | |
| 224 | CVE: CVE-2022-33065 |
| 225 | Fixes: https://github.com/libsndfile/libsndfile/issues/833 |
| 226 | |
| 227 | Signed-off-by: Alex Stewart <alex.stewart@ni.com> |
| 228 | --- |
| 229 | src/common.h | 2 +- |
| 230 | src/ircam.c | 10 +++++----- |
| 231 | 2 files changed, 6 insertions(+), 6 deletions(-) |
| 232 | |
| 233 | diff --git a/src/common.h b/src/common.h |
| 234 | index d92eabde..5369cb67 100644 |
| 235 | --- a/src/common.h |
| 236 | +++ b/src/common.h |
| 237 | @@ -439,7 +439,7 @@ typedef struct sf_private_tag |
| 238 | sf_count_t datalength ; /* Length in bytes of the audio data. */ |
| 239 | sf_count_t dataend ; /* Offset to file tailer. */ |
| 240 | |
| 241 | - int blockwidth ; /* Size in bytes of one set of interleaved samples. */ |
| 242 | + sf_count_t blockwidth ; /* Size in bytes of one set of interleaved samples. */ |
| 243 | int bytewidth ; /* Size in bytes of one sample (one channel). */ |
| 244 | |
| 245 | void *dither ; |
| 246 | diff --git a/src/ircam.c b/src/ircam.c |
| 247 | index 8e7cdba8..3d73ba44 100644 |
| 248 | --- a/src/ircam.c |
| 249 | +++ b/src/ircam.c |
| 250 | @@ -171,35 +171,35 @@ ircam_read_header (SF_PRIVATE *psf) |
| 251 | switch (encoding) |
| 252 | { case IRCAM_PCM_16 : |
| 253 | psf->bytewidth = 2 ; |
| 254 | - psf->blockwidth = psf->sf.channels * psf->bytewidth ; |
| 255 | + psf->blockwidth = (sf_count_t) psf->sf.channels * psf->bytewidth ; |
| 256 | |
| 257 | psf->sf.format = SF_FORMAT_IRCAM | SF_FORMAT_PCM_16 ; |
| 258 | break ; |
| 259 | |
| 260 | case IRCAM_PCM_32 : |
| 261 | psf->bytewidth = 4 ; |
| 262 | - psf->blockwidth = psf->sf.channels * psf->bytewidth ; |
| 263 | + psf->blockwidth = (sf_count_t) psf->sf.channels * psf->bytewidth ; |
| 264 | |
| 265 | psf->sf.format = SF_FORMAT_IRCAM | SF_FORMAT_PCM_32 ; |
| 266 | break ; |
| 267 | |
| 268 | case IRCAM_FLOAT : |
| 269 | psf->bytewidth = 4 ; |
| 270 | - psf->blockwidth = psf->sf.channels * psf->bytewidth ; |
| 271 | + psf->blockwidth = (sf_count_t) psf->sf.channels * psf->bytewidth ; |
| 272 | |
| 273 | psf->sf.format = SF_FORMAT_IRCAM | SF_FORMAT_FLOAT ; |
| 274 | break ; |
| 275 | |
| 276 | case IRCAM_ALAW : |
| 277 | psf->bytewidth = 1 ; |
| 278 | - psf->blockwidth = psf->sf.channels * psf->bytewidth ; |
| 279 | + psf->blockwidth = (sf_count_t) psf->sf.channels * psf->bytewidth ; |
| 280 | |
| 281 | psf->sf.format = SF_FORMAT_IRCAM | SF_FORMAT_ALAW ; |
| 282 | break ; |
| 283 | |
| 284 | case IRCAM_ULAW : |
| 285 | psf->bytewidth = 1 ; |
| 286 | - psf->blockwidth = psf->sf.channels * psf->bytewidth ; |
| 287 | + psf->blockwidth = (sf_count_t) psf->sf.channels * psf->bytewidth ; |
| 288 | |
| 289 | psf->sf.format = SF_FORMAT_IRCAM | SF_FORMAT_ULAW ; |
| 290 | break ; |
| 291 | From 3bcd291e57867f88f558fa6f80990e84311df78c Mon Sep 17 00:00:00 2001 |
| 292 | From: Alex Stewart <alex.stewart@ni.com> |
| 293 | Date: Wed, 11 Oct 2023 16:12:22 -0400 |
| 294 | Subject: [PATCH] mat4/mat5: fix int overflow when calculating blockwidth |
| 295 | |
| 296 | Pre-cast the components of the blockwidth calculation to sf_count_t to |
| 297 | avoid overflowing integers during calculation. |
| 298 | |
| 299 | CVE: CVE-2022-33065 |
| 300 | Fixes: https://github.com/libsndfile/libsndfile/issues/833 |
| 301 | |
| 302 | Signed-off-by: Alex Stewart <alex.stewart@ni.com> |
| 303 | --- |
| 304 | src/mat4.c | 2 +- |
| 305 | src/mat5.c | 2 +- |
| 306 | 2 files changed, 2 insertions(+), 2 deletions(-) |
| 307 | |
| 308 | diff --git a/src/mat4.c b/src/mat4.c |
| 309 | index 575683ba..9f046f0c 100644 |
| 310 | --- a/src/mat4.c |
| 311 | +++ b/src/mat4.c |
| 312 | @@ -104,7 +104,7 @@ mat4_open (SF_PRIVATE *psf) |
| 313 | |
| 314 | psf->container_close = mat4_close ; |
| 315 | |
| 316 | - psf->blockwidth = psf->bytewidth * psf->sf.channels ; |
| 317 | + psf->blockwidth = (sf_count_t) psf->bytewidth * psf->sf.channels ; |
| 318 | |
| 319 | switch (subformat) |
| 320 | { case SF_FORMAT_PCM_16 : |
| 321 | diff --git a/src/mat5.c b/src/mat5.c |
| 322 | index da5a6eca..20f0ea64 100644 |
| 323 | --- a/src/mat5.c |
| 324 | +++ b/src/mat5.c |
| 325 | @@ -114,7 +114,7 @@ mat5_open (SF_PRIVATE *psf) |
| 326 | |
| 327 | psf->container_close = mat5_close ; |
| 328 | |
| 329 | - psf->blockwidth = psf->bytewidth * psf->sf.channels ; |
| 330 | + psf->blockwidth = (sf_count_t) psf->bytewidth * psf->sf.channels ; |
| 331 | |
| 332 | switch (subformat) |
| 333 | { case SF_FORMAT_PCM_U8 : |
| 334 | From c177e292d47ef73b1d3c1bb391320299a0ed2ff9 Mon Sep 17 00:00:00 2001 |
| 335 | From: Alex Stewart <alex.stewart@ni.com> |
| 336 | Date: Mon, 16 Oct 2023 12:37:47 -0400 |
| 337 | Subject: [PATCH] common: fix int overflow in psf_binheader_readf() |
| 338 | |
| 339 | The psf_binheader_readf() function attempts to count and return the |
| 340 | number of bytes traversed in the header. During this accumulation, it is |
| 341 | possible to overflow the int-sized byte_count variable. |
| 342 | |
| 343 | Avoid this overflow by checking that the accumulated bytes do not exceed |
| 344 | INT_MAX and throwing an error if they do. This implies that files with |
| 345 | multi-gigabyte headers threaten to produce this error, but I imagine |
| 346 | those files don't really exist - and this error is better than the |
| 347 | undefined behavior which would have resulted previously. |
| 348 | |
| 349 | CVE: CVE-2022-33065 |
| 350 | Fixes: https://github.com/libsndfile/libsndfile/issues/833 |
| 351 | |
| 352 | Signed-off-by: Alex Stewart <alex.stewart@ni.com> |
| 353 | --- |
| 354 | src/common.c | 36 ++++++++++++++++++++++++------------ |
| 355 | 1 file changed, 24 insertions(+), 12 deletions(-) |
| 356 | |
| 357 | diff --git a/src/common.c b/src/common.c |
| 358 | index 1c3d951d..7f6cceca 100644 |
| 359 | --- a/src/common.c |
| 360 | +++ b/src/common.c |
| 361 | @@ -18,6 +18,7 @@ |
| 362 | |
| 363 | #include <config.h> |
| 364 | |
| 365 | +#include <limits.h> |
| 366 | #include <stdarg.h> |
| 367 | #include <string.h> |
| 368 | #if HAVE_UNISTD_H |
| 369 | @@ -990,6 +991,7 @@ psf_binheader_readf (SF_PRIVATE *psf, char const *format, ...) |
| 370 | double *doubleptr ; |
| 371 | char c ; |
| 372 | int byte_count = 0, count = 0 ; |
| 373 | + int read_bytes = 0 ; |
| 374 | |
| 375 | if (! format) |
| 376 | return psf_ftell (psf) ; |
| 377 | @@ -998,6 +1000,7 @@ psf_binheader_readf (SF_PRIVATE *psf, char const *format, ...) |
| 378 | |
| 379 | while ((c = *format++)) |
| 380 | { |
| 381 | + read_bytes = 0 ; |
| 382 | if (psf->header.indx + 16 >= psf->header.len && psf_bump_header_allocation (psf, 16)) |
| 383 | break ; |
| 384 | |
| 385 | @@ -1014,7 +1017,7 @@ psf_binheader_readf (SF_PRIVATE *psf, char const *format, ...) |
| 386 | intptr = va_arg (argptr, unsigned int*) ; |
| 387 | *intptr = 0 ; |
| 388 | ucptr = (unsigned char*) intptr ; |
| 389 | - byte_count += header_read (psf, ucptr, sizeof (int)) ; |
| 390 | + read_bytes = header_read (psf, ucptr, sizeof (int)) ; |
| 391 | *intptr = GET_MARKER (ucptr) ; |
| 392 | break ; |
| 393 | |
| 394 | @@ -1022,7 +1025,7 @@ psf_binheader_readf (SF_PRIVATE *psf, char const *format, ...) |
| 395 | intptr = va_arg (argptr, unsigned int*) ; |
| 396 | *intptr = 0 ; |
| 397 | ucptr = (unsigned char*) intptr ; |
| 398 | - byte_count += header_read (psf, sixteen_bytes, sizeof (sixteen_bytes)) ; |
| 399 | + read_bytes = header_read (psf, sixteen_bytes, sizeof (sixteen_bytes)) ; |
| 400 | { int k ; |
| 401 | intdata = 0 ; |
| 402 | for (k = 0 ; k < 16 ; k++) |
| 403 | @@ -1034,14 +1037,14 @@ psf_binheader_readf (SF_PRIVATE *psf, char const *format, ...) |
| 404 | case '1' : |
| 405 | charptr = va_arg (argptr, char*) ; |
| 406 | *charptr = 0 ; |
| 407 | - byte_count += header_read (psf, charptr, sizeof (char)) ; |
| 408 | + read_bytes = header_read (psf, charptr, sizeof (char)) ; |
| 409 | break ; |
| 410 | |
| 411 | case '2' : /* 2 byte value with the current endian-ness */ |
| 412 | shortptr = va_arg (argptr, unsigned short*) ; |
| 413 | *shortptr = 0 ; |
| 414 | ucptr = (unsigned char*) shortptr ; |
| 415 | - byte_count += header_read (psf, ucptr, sizeof (short)) ; |
| 416 | + read_bytes = header_read (psf, ucptr, sizeof (short)) ; |
| 417 | if (psf->rwf_endian == SF_ENDIAN_BIG) |
| 418 | *shortptr = GET_BE_SHORT (ucptr) ; |
| 419 | else |
| 420 | @@ -1051,7 +1054,7 @@ psf_binheader_readf (SF_PRIVATE *psf, char const *format, ...) |
| 421 | case '3' : /* 3 byte value with the current endian-ness */ |
| 422 | intptr = va_arg (argptr, unsigned int*) ; |
| 423 | *intptr = 0 ; |
| 424 | - byte_count += header_read (psf, sixteen_bytes, 3) ; |
| 425 | + read_bytes = header_read (psf, sixteen_bytes, 3) ; |
| 426 | if (psf->rwf_endian == SF_ENDIAN_BIG) |
| 427 | *intptr = GET_BE_3BYTE (sixteen_bytes) ; |
| 428 | else |
| 429 | @@ -1062,7 +1065,7 @@ psf_binheader_readf (SF_PRIVATE *psf, char const *format, ...) |
| 430 | intptr = va_arg (argptr, unsigned int*) ; |
| 431 | *intptr = 0 ; |
| 432 | ucptr = (unsigned char*) intptr ; |
| 433 | - byte_count += header_read (psf, ucptr, sizeof (int)) ; |
| 434 | + read_bytes = header_read (psf, ucptr, sizeof (int)) ; |
| 435 | if (psf->rwf_endian == SF_ENDIAN_BIG) |
| 436 | *intptr = psf_get_be32 (ucptr, 0) ; |
| 437 | else |
| 438 | @@ -1072,7 +1075,7 @@ psf_binheader_readf (SF_PRIVATE *psf, char const *format, ...) |
| 439 | case '8' : /* 8 byte value with the current endian-ness */ |
| 440 | countptr = va_arg (argptr, sf_count_t *) ; |
| 441 | *countptr = 0 ; |
| 442 | - byte_count += header_read (psf, sixteen_bytes, 8) ; |
| 443 | + read_bytes = header_read (psf, sixteen_bytes, 8) ; |
| 444 | if (psf->rwf_endian == SF_ENDIAN_BIG) |
| 445 | countdata = psf_get_be64 (sixteen_bytes, 0) ; |
| 446 | else |
| 447 | @@ -1083,7 +1086,7 @@ psf_binheader_readf (SF_PRIVATE *psf, char const *format, ...) |
| 448 | case 'f' : /* Float conversion */ |
| 449 | floatptr = va_arg (argptr, float *) ; |
| 450 | *floatptr = 0.0 ; |
| 451 | - byte_count += header_read (psf, floatptr, sizeof (float)) ; |
| 452 | + read_bytes = header_read (psf, floatptr, sizeof (float)) ; |
| 453 | if (psf->rwf_endian == SF_ENDIAN_BIG) |
| 454 | *floatptr = float32_be_read ((unsigned char*) floatptr) ; |
| 455 | else |
| 456 | @@ -1093,7 +1096,7 @@ psf_binheader_readf (SF_PRIVATE *psf, char const *format, ...) |
| 457 | case 'd' : /* double conversion */ |
| 458 | doubleptr = va_arg (argptr, double *) ; |
| 459 | *doubleptr = 0.0 ; |
| 460 | - byte_count += header_read (psf, doubleptr, sizeof (double)) ; |
| 461 | + read_bytes = header_read (psf, doubleptr, sizeof (double)) ; |
| 462 | if (psf->rwf_endian == SF_ENDIAN_BIG) |
| 463 | *doubleptr = double64_be_read ((unsigned char*) doubleptr) ; |
| 464 | else |
| 465 | @@ -1117,7 +1120,7 @@ psf_binheader_readf (SF_PRIVATE *psf, char const *format, ...) |
| 466 | charptr = va_arg (argptr, char*) ; |
| 467 | count = va_arg (argptr, size_t) ; |
| 468 | memset (charptr, 0, count) ; |
| 469 | - byte_count += header_read (psf, charptr, count) ; |
| 470 | + read_bytes = header_read (psf, charptr, count) ; |
| 471 | break ; |
| 472 | |
| 473 | case 'G' : |
| 474 | @@ -1128,7 +1131,7 @@ psf_binheader_readf (SF_PRIVATE *psf, char const *format, ...) |
| 475 | if (psf->header.indx + count >= psf->header.len && psf_bump_header_allocation (psf, count)) |
| 476 | break ; |
| 477 | |
| 478 | - byte_count += header_gets (psf, charptr, count) ; |
| 479 | + read_bytes = header_gets (psf, charptr, count) ; |
| 480 | break ; |
| 481 | |
| 482 | case 'z' : |
| 483 | @@ -1152,7 +1155,7 @@ psf_binheader_readf (SF_PRIVATE *psf, char const *format, ...) |
| 484 | case 'j' : /* Seek to position from current position. */ |
| 485 | count = va_arg (argptr, size_t) ; |
| 486 | header_seek (psf, count, SEEK_CUR) ; |
| 487 | - byte_count += count ; |
| 488 | + read_bytes = count ; |
| 489 | break ; |
| 490 | |
| 491 | case '!' : /* Clear buffer, forcing re-read. */ |
| 492 | @@ -1164,8 +1167,17 @@ psf_binheader_readf (SF_PRIVATE *psf, char const *format, ...) |
| 493 | psf->error = SFE_INTERNAL ; |
| 494 | break ; |
| 495 | } ; |
| 496 | + |
| 497 | + if (read_bytes > 0 && byte_count > (INT_MAX - read_bytes)) |
| 498 | + { psf_log_printf (psf, "Header size exceeds INT_MAX. Aborting.", c) ; |
| 499 | + psf->error = SFE_INTERNAL ; |
| 500 | + break ; |
| 501 | + } else |
| 502 | + { byte_count += read_bytes ; |
| 503 | } ; |
| 504 | |
| 505 | + } ; /*end while*/ |
| 506 | + |
| 507 | va_end (argptr) ; |
| 508 | |
| 509 | return byte_count ; |
| 510 | From a23d563386e7c8d93dcdbe7d5b1d63cad6009116 Mon Sep 17 00:00:00 2001 |
| 511 | From: Alex Stewart <alex.stewart@ni.com> |
| 512 | Date: Thu, 19 Oct 2023 14:07:19 -0400 |
| 513 | Subject: [PATCH] nms_adpcm: fix int overflow in signal estimate |
| 514 | |
| 515 | It is possible (though functionally incorrect) for the signal estimate |
| 516 | calculation in nms_adpcm_update() to overflow the int value of s_e, |
| 517 | resulting in undefined behavior. |
| 518 | |
| 519 | Since adpcm state signal values are never practically larger than |
| 520 | 16 bits, use smaller numeric sizes throughout the file to avoid the |
| 521 | overflow. |
| 522 | |
| 523 | CVE: CVE-2022-33065 |
| 524 | Fixes: https://github.com/libsndfile/libsndfile/issues/833 |
| 525 | |
| 526 | Authored-by: Arthur Taylor <art@ified.ca> |
| 527 | Signed-off-by: Alex Stewart <alex.stewart@ni.com> |
| 528 | Rebased-by: Alex Stewart <alex.stewart@ni.com> |
| 529 | --- |
| 530 | src/nms_adpcm.c | 85 ++++++++++++++++++++++++------------------------- |
| 531 | 1 file changed, 42 insertions(+), 43 deletions(-) |
| 532 | |
| 533 | diff --git a/src/nms_adpcm.c b/src/nms_adpcm.c |
| 534 | index 96d6ad26..460ea077 100644 |
| 535 | --- a/src/nms_adpcm.c |
| 536 | +++ b/src/nms_adpcm.c |
| 537 | @@ -48,36 +48,36 @@ |
| 538 | /* Variable names from ITU G.726 spec */ |
| 539 | struct nms_adpcm_state |
| 540 | { /* Log of the step size multiplier. Operated on by codewords. */ |
| 541 | - int yl ; |
| 542 | + short yl ; |
| 543 | |
| 544 | /* Quantizer step size multiplier. Generated from yl. */ |
| 545 | - int y ; |
| 546 | + short y ; |
| 547 | |
| 548 | - /* Coefficents of the pole predictor */ |
| 549 | - int a [2] ; |
| 550 | + /* Coefficients of the pole predictor */ |
| 551 | + short a [2] ; |
| 552 | |
| 553 | - /* Coefficents of the zero predictor */ |
| 554 | - int b [6] ; |
| 555 | + /* Coefficients of the zero predictor */ |
| 556 | + short b [6] ; |
| 557 | |
| 558 | /* Previous quantized deltas (multiplied by 2^14) */ |
| 559 | - int d_q [7] ; |
| 560 | + short d_q [7] ; |
| 561 | |
| 562 | /* d_q [x] + s_ez [x], used by the pole-predictor for signs only. */ |
| 563 | - int p [3] ; |
| 564 | + short p [3] ; |
| 565 | |
| 566 | /* Previous reconstructed signal values. */ |
| 567 | - int s_r [2] ; |
| 568 | + short s_r [2] ; |
| 569 | |
| 570 | /* Zero predictor components of the signal estimate. */ |
| 571 | - int s_ez ; |
| 572 | + short s_ez ; |
| 573 | |
| 574 | /* Signal estimate, (including s_ez). */ |
| 575 | - int s_e ; |
| 576 | + short s_e ; |
| 577 | |
| 578 | /* The most recent codeword (enc:generated, dec:inputted) */ |
| 579 | - int Ik ; |
| 580 | + char Ik ; |
| 581 | |
| 582 | - int parity ; |
| 583 | + char parity ; |
| 584 | |
| 585 | /* |
| 586 | ** Offset into code tables for the bitrate. |
| 587 | @@ -109,7 +109,7 @@ typedef struct |
| 588 | } NMS_ADPCM_PRIVATE ; |
| 589 | |
| 590 | /* Pre-computed exponential interval used in the antilog approximation. */ |
| 591 | -static unsigned int table_expn [] = |
| 592 | +static unsigned short table_expn [] = |
| 593 | { 0x4000, 0x4167, 0x42d5, 0x444c, 0x45cb, 0x4752, 0x48e2, 0x4a7a, |
| 594 | 0x4c1b, 0x4dc7, 0x4f7a, 0x5138, 0x52ff, 0x54d1, 0x56ac, 0x5892, |
| 595 | 0x5a82, 0x5c7e, 0x5e84, 0x6096, 0x62b4, 0x64dd, 0x6712, 0x6954, |
| 596 | @@ -117,21 +117,21 @@ static unsigned int table_expn [] = |
| 597 | } ; |
| 598 | |
| 599 | /* Table mapping codewords to scale factor deltas. */ |
| 600 | -static int table_scale_factor_step [] = |
| 601 | +static short table_scale_factor_step [] = |
| 602 | { 0x0, 0x0, 0x0, 0x0, 0x4b0, 0x0, 0x0, 0x0, /* 2-bit */ |
| 603 | -0x3c, 0x0, 0x90, 0x0, 0x2ee, 0x0, 0x898, 0x0, /* 3-bit */ |
| 604 | -0x30, 0x12, 0x6b, 0xc8, 0x188, 0x2e0, 0x551, 0x1150, /* 4-bit */ |
| 605 | } ; |
| 606 | |
| 607 | /* Table mapping codewords to quantized delta interval steps. */ |
| 608 | -static unsigned int table_step [] = |
| 609 | +static unsigned short table_step [] = |
| 610 | { 0x73F, 0, 0, 0, 0x1829, 0, 0, 0, /* 2-bit */ |
| 611 | 0x3EB, 0, 0xC18, 0, 0x1581, 0, 0x226E, 0, /* 3-bit */ |
| 612 | 0x20C, 0x635, 0xA83, 0xF12, 0x1418, 0x19E3, 0x211A, 0x2BBA, /* 4-bit */ |
| 613 | } ; |
| 614 | |
| 615 | /* Binary search lookup table for quantizing using table_step. */ |
| 616 | -static int table_step_search [] = |
| 617 | +static short table_step_search [] = |
| 618 | { 0, 0x1F6D, 0, -0x1F6D, 0, 0, 0, 0, /* 2-bit */ |
| 619 | 0x1008, 0x1192, 0, -0x219A, 0x1656, -0x1656, 0, 0, /* 3-bit */ |
| 620 | 0x872, 0x1277, -0x8E6, -0x232B, 0xD06, -0x17D7, -0x11D3, 0, /* 4-bit */ |
| 621 | @@ -179,23 +179,23 @@ static sf_count_t nms_adpcm_seek (SF_PRIVATE *psf, int mode, sf_count_t offset) |
| 622 | ** Maps [1,20480] to [1,1024] in an exponential relationship. This is |
| 623 | ** approximately ret = b^exp where b = e^(ln(1024)/ln(20480)) ~= 1.0003385 |
| 624 | */ |
| 625 | -static inline int |
| 626 | -nms_adpcm_antilog (int exp) |
| 627 | -{ int ret ; |
| 628 | +static inline short |
| 629 | +nms_adpcm_antilog (short exp) |
| 630 | +{ int_fast32_t r ; |
| 631 | |
| 632 | - ret = 0x1000 ; |
| 633 | - ret += (((exp & 0x3f) * 0x166b) >> 12) ; |
| 634 | - ret *= table_expn [(exp & 0x7c0) >> 6] ; |
| 635 | - ret >>= (26 - (exp >> 11)) ; |
| 636 | + r = 0x1000 ; |
| 637 | + r += (((int_fast32_t) (exp & 0x3f) * 0x166b) >> 12) ; |
| 638 | + r *= table_expn [(exp & 0x7c0) >> 6] ; |
| 639 | + r >>= (26 - (exp >> 11)) ; |
| 640 | |
| 641 | - return ret ; |
| 642 | + return (short) r ; |
| 643 | } /* nms_adpcm_antilog */ |
| 644 | |
| 645 | static void |
| 646 | nms_adpcm_update (struct nms_adpcm_state *s) |
| 647 | { /* Variable names from ITU G.726 spec */ |
| 648 | - int a1ul ; |
| 649 | - int fa1 ; |
| 650 | + short a1ul, fa1 ; |
| 651 | + int_fast32_t se ; |
| 652 | int i ; |
| 653 | |
| 654 | /* Decay and Modify the scale factor in the log domain based on the codeword. */ |
| 655 | @@ -222,7 +222,7 @@ nms_adpcm_update (struct nms_adpcm_state *s) |
| 656 | else if (fa1 > 256) |
| 657 | fa1 = 256 ; |
| 658 | |
| 659 | - s->a [0] = (0xff * s->a [0]) >> 8 ; |
| 660 | + s->a [0] = (s->a [0] * 0xff) >> 8 ; |
| 661 | if (s->p [0] != 0 && s->p [1] != 0 && ((s->p [0] ^ s->p [1]) < 0)) |
| 662 | s->a [0] -= 192 ; |
| 663 | else |
| 664 | @@ -230,7 +230,7 @@ nms_adpcm_update (struct nms_adpcm_state *s) |
| 665 | fa1 = -fa1 ; |
| 666 | } |
| 667 | |
| 668 | - s->a [1] = fa1 + ((0xfe * s->a [1]) >> 8) ; |
| 669 | + s->a [1] = fa1 + ((s->a [1] * 0xfe) >> 8) ; |
| 670 | if (s->p [0] != 0 && s->p [2] != 0 && ((s->p [0] ^ s->p [2]) < 0)) |
| 671 | s->a [1] -= 128 ; |
| 672 | else |
| 673 | @@ -250,19 +250,18 @@ nms_adpcm_update (struct nms_adpcm_state *s) |
| 674 | s->a [0] = a1ul ; |
| 675 | } ; |
| 676 | |
| 677 | - /* Compute the zero predictor estimate. Rotate past deltas too. */ |
| 678 | - s->s_ez = 0 ; |
| 679 | + /* Compute the zero predictor estimate and rotate past deltas. */ |
| 680 | + se = 0 ; |
| 681 | for (i = 5 ; i >= 0 ; i--) |
| 682 | - { s->s_ez += s->d_q [i] * s->b [i] ; |
| 683 | + { se += (int_fast32_t) s->d_q [i] * s->b [i] ; |
| 684 | s->d_q [i + 1] = s->d_q [i] ; |
| 685 | } ; |
| 686 | + s->s_ez = se >> 14 ; |
| 687 | |
| 688 | - /* Compute the signal estimate. */ |
| 689 | - s->s_e = s->a [0] * s->s_r [0] + s->a [1] * s->s_r [1] + s->s_ez ; |
| 690 | - |
| 691 | - /* Return to scale */ |
| 692 | - s->s_ez >>= 14 ; |
| 693 | - s->s_e >>= 14 ; |
| 694 | + /* Complete the signal estimate. */ |
| 695 | + se += (int_fast32_t) s->a [0] * s->s_r [0] ; |
| 696 | + se += (int_fast32_t) s->a [1] * s->s_r [1] ; |
| 697 | + s->s_e = se >> 14 ; |
| 698 | |
| 699 | /* Rotate members to prepare for next iteration. */ |
| 700 | s->s_r [1] = s->s_r [0] ; |
| 701 | @@ -274,7 +273,7 @@ nms_adpcm_update (struct nms_adpcm_state *s) |
| 702 | static int16_t |
| 703 | nms_adpcm_reconstruct_sample (struct nms_adpcm_state *s, uint8_t I) |
| 704 | { /* Variable names from ITU G.726 spec */ |
| 705 | - int dqx ; |
| 706 | + int_fast32_t dqx ; |
| 707 | |
| 708 | /* |
| 709 | ** The ordering of the 12-bit right-shift is a precision loss. It agrees |
| 710 | @@ -308,17 +307,17 @@ nms_adpcm_codec_init (struct nms_adpcm_state *s, enum nms_enc_type type) |
| 711 | /* |
| 712 | ** nms_adpcm_encode_sample() |
| 713 | ** |
| 714 | -** Encode a linear 16-bit pcm sample into a 2,3, or 4 bit NMS-ADPCM codeword |
| 715 | +** Encode a linear 16-bit pcm sample into a 2, 3, or 4 bit NMS-ADPCM codeword |
| 716 | ** using and updating the predictor state. |
| 717 | */ |
| 718 | static uint8_t |
| 719 | nms_adpcm_encode_sample (struct nms_adpcm_state *s, int16_t sl) |
| 720 | { /* Variable names from ITU G.726 spec */ |
| 721 | - int d ; |
| 722 | + int_fast32_t d ; |
| 723 | uint8_t I ; |
| 724 | |
| 725 | /* Down scale the sample from 16 => ~14 bits. */ |
| 726 | - sl = (sl * 0x1fdf) / 0x7fff ; |
| 727 | + sl = ((int_fast32_t) sl * 0x1fdf) / 0x7fff ; |
| 728 | |
| 729 | /* Compute estimate, and delta from actual value */ |
| 730 | nms_adpcm_update (s) ; |
| 731 | @@ -407,7 +406,7 @@ nms_adpcm_encode_sample (struct nms_adpcm_state *s, int16_t sl) |
| 732 | */ |
| 733 | static int16_t |
| 734 | nms_adpcm_decode_sample (struct nms_adpcm_state *s, uint8_t I) |
| 735 | -{ int sl ; |
| 736 | +{ int_fast32_t sl ; |
| 737 | |
| 738 | nms_adpcm_update (s) ; |
| 739 | sl = nms_adpcm_reconstruct_sample (s, I) ; |