Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

type confusion(double's NAN & INF) vuls was found in function sfe_copy_data_fp() #317

Closed
Xin-Jiang opened this issue Sep 12, 2017 · 8 comments

Comments

Projects
None yet
8 participants
@Xin-Jiang
Copy link

commented Sep 12, 2017

Some type confusion vuls was found in function sfe_copy_data_fp() in programs/common.c of libsndfile-1.0.28:
sfe_copy_data_fp (SNDFILE *outfile, SNDFILE *infile, int channels, int normalize)
{ static double data [BUFFER_LEN], max ;
int frames, readcount, k ;

frames = BUFFER_LEN / channels ;
readcount = frames ;

sf_command (infile, SFC_CALC_SIGNAL_MAX, &max, sizeof (max)) ;

if (!normalize && max < 1.0)
{	while (readcount > 0)
	{	readcount = sf_readf_double (infile, data, frames) ;
		sf_writef_double (outfile, data, readcount) ;
		} ;
	}
else
{	sf_command (infile, SFC_SET_NORM_DOUBLE, NULL, SF_FALSE) ;

	while (readcount > 0)
	{	readcount = sf_readf_double (infile, data, frames) ;//can read from host_read_d(), so data can be controled by user.
		for (k = 0 ; k < readcount * channels ; k++)
			data [k] /= max ;
		sf_writef_double (outfile, data, readcount) ;
		} ;
} ;

return ;

} /* sfe_copy_data_fp */

There are two ways to generate NAN or INF double type by a craft audio file. first, the value of max may be 0 or inf, so it will subsequently get a NAN while "data[k]/=max"; second , the data[] may came from audio file directly, so data[k] may be NAN or INF and so does "data [k] /= max".
Then, while d2ulaw_array()/f2ulaw_array()/d2alaw_array()/f2alaw_array() in ulaw.c and alaw.c, a SEGV will appear:

$ gdb programs/sndfile-convert
GNU gdb (GDB) 7.9
Copyright (C) 2015 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later http://gnu.org/licenses/gpl.html
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law. Type "show copying"
and "show warranty" for details.
This GDB was configured as "x86_64-unknown-linux-gnu".
Type "show configuration" for configuration details.
For bug reporting instructions, please see:
http://www.gnu.org/software/gdb/bugs/.
Find the GDB manual and other documentation resources online at:
http://www.gnu.org/software/gdb/documentation/.
For help, type "help".
Type "apropos word" to search for commands related to "word"...
Reading symbols from programs/sndfile-convert...done.
(gdb) run -ulaw ../../libsndfile-1.0.28/fuzz/crash1-get-nan-from-host xxx.vox
Starting program: /home/jiangxin/experiment/fuzz/AFL/target/debug/libsndfile-1.0.28/programs/sndfile-convert -ulaw ../../libsndfile-1.0.28/fuzz/crash1-get-nan-from-host xxx.vox

Program received signal SIGSEGV, Segmentation fault.
0x0000000000453781 in d2ulaw_array (ptr=0x68ba20 , count=4, buffer=0x7fffffffc010 "1\005\352\177J\377\200\200\200\200\377\377", 'J' <repeats 12 times>, "1\261\061\061\061\061\061\061", normfact=8191.75) at ulaw.c:853
853 buffer [count] = 0x7F & ulaw_encode [- lrint (normfact * ptr [count])] ;
(gdb) bt
#0 0x0000000000453781 in d2ulaw_array (ptr=0x68ba20 , count=4, buffer=0x7fffffffc010 "1\005\352\177J\377\200\200\200\200\377\377", 'J' <repeats 12 times>, "1\261\061\061\061\061\061\061", normfact=8191.75) at ulaw.c:853
#1 0x0000000000453e89 in ulaw_write_d2ulaw (psf=0x69d2f0, ptr=0x68ba20 , len=12) at ulaw.c:1041
#2 0x0000000000409a7c in sf_writef_double (sndfile=0x69d2f0, ptr=0x68ba20 , frames=12) at sndfile.c:2570
#3 0x00000000004024a1 in sfe_copy_data_fp (outfile=0x69d2f0, infile=0x69a010, channels=1, normalize=0) at common.c:79
#4 0x0000000000402122 in main (argc=4, argv=0x7fffffffe228) at sndfile-convert.c:338
(gdb) p ptr[count]
$1 = nan(0xd40a620000000)
(gdb) i r
rax 0x8000000000000000 -9223372036854775808
rbx 0x7fffffffc014 140737488338964
rcx 0xc 12
rdx 0x20 32
rsi 0xc 12
rdi 0x68ba20 6863392
rbp 0x7fffffffbfe0 0x7fffffffbfe0
rsp 0x7fffffffbfb0 0x7fffffffbfb0
r8 0x0 0
r9 0x100000 1048576
r10 0x0 0
r11 0x246 582
r12 0x4015a0 4199840
r13 0x7fffffffe220 140737488347680
r14 0x0 0
r15 0x0 0
rip 0x453781 0x453781 <d2ulaw_array+174>
eflags 0x10a87 [ CF PF SF IF OF RF ]
cs 0x33 51
ss 0x2b 43
ds 0x0 0
es 0x0 0
fs 0x0 0
gs 0x0 0
(gdb) x/i $pc
=> 0x453781 <d2ulaw_array+174>: movzbl 0x689760(%rax),%eax
(gdb) detach
Detaching from program: /home/jiangxin/experiment/fuzz/AFL/target/debug/libsndfile-1.0.28/programs/sndfile-convert, process 14340
(gdb) q

@Xin-Jiang Xin-Jiang closed this Sep 12, 2017

@Xin-Jiang Xin-Jiang reopened this Sep 12, 2017

@Xin-Jiang Xin-Jiang changed the title DoubleType confusion in type confusion(double's NAN & INF) in function sfe_copy_data_fp() Sep 12, 2017

@Xin-Jiang

This comment has been minimized.

Copy link
Author

commented Sep 12, 2017

to reproduce these vuls, try "sndfile-convert -normalize -ulaw/-alaw samples out.vox", while samples attached as belows:

samples.zip

@Xin-Jiang Xin-Jiang changed the title type confusion(double's NAN & INF) in function sfe_copy_data_fp() type confusion(double's NAN & INF) vuls was found in function sfe_copy_data_fp() Sep 12, 2017

@fabiangreffrath

This comment has been minimized.

Copy link
Contributor

commented Sep 27, 2017

Is libsndfile supposed to be built by a C99-compliant compiler? If yes, some isfinite() or isnormal() checks could get added here to cover these cases.

fabiangreffrath added a commit to fabiangreffrath/libsndfile that referenced this issue Sep 27, 2017

sfe_copy_data_fp: check value of "max" variable for being normal
and check elements of the data[] array for being finite.

Both checks use functions provided by the <math.h> header as declared
by the C99 standard.

Fixes erikd#317
CVE-2017-14245
CVE-2017-14246
@carnil

This comment has been minimized.

Copy link

commented Nov 26, 2017

Where there any news on adressing these two issues?

@erikd

This comment has been minimized.

Copy link
Owner

commented Nov 26, 2017

Not yet. The proposed fix in PR #325 only papers over the symptoms. I haven't had time to figure out a better fix.

@hlef

This comment has been minimized.

Copy link
Contributor

commented Dec 24, 2018

@erikd also fixed by 8ddc442 (#344 and #317 are the same)

@rossburton

This comment has been minimized.

Copy link

commented Mar 5, 2019

@erikd would you be able to verify the assertion by @hlef that #344 and #317 are the same and close this too? #344 is closed as a duplicate of this, but the 8ddc442 refers to #344 and those CVEs, not the ones in this entry.

@erikd

This comment has been minimized.

Copy link
Owner

commented Mar 10, 2019

#344 and #317 are both fixed in git HEAD.

@erikd erikd closed this Mar 10, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.