shithub: libsamplerate

Download patch

ref: c10c8191f21dd12c1bea2d9b9a68536f7cb0491c
parent: 8c325e35494ba9d2a5578d4a6702691884b85f0f
author: Erik de Castro Lopo <erikd@mega-nerd.com>
date: Sat Aug 24 11:13:32 EDT 2019 

src/src_sinc.c: Fix a buffer out-of-bounds read error

The buffer out-of-bounds read that happens with the 'SRC_SINC_*' converters
when the `src_ratio` is dynamically decreased while processing.

This is a relatively naive fix for issue that seems to have an up to 3%
performance degradation with respect to the unfixed version. It may be
possible to come up with a better version of this fix that does not
degrade performance.

Closes: https://github.com/erikd/libsamplerate/issues/5


--- a/src/src_sinc.c
+++ b/src/src_sinc.c
@@ -295,12 +295,14 @@
 
 	left = 0.0 ;
 	do
-	{	fraction = fp_to_double (filter_index) ;
-		indx = fp_to_int (filter_index) ;
+	{	if (data_index >= 0) /* Avoid underflow access to filter->buffer. */
+		{	fraction = fp_to_double (filter_index) ;
+			indx = fp_to_int (filter_index) ;
 
-		icoeff = filter->coeffs [indx] + fraction * (filter->coeffs [indx + 1] - filter->coeffs [indx]) ;
+			icoeff = filter->coeffs [indx] + fraction * (filter->coeffs [indx + 1] - filter->coeffs [indx]) ;
 
-		left += icoeff * filter->buffer [data_index] ;
+			left += icoeff * filter->buffer [data_index] ;
+			}  ;
 
 		filter_index -= increment ;
 		data_index = data_index + 1 ;
@@ -440,13 +442,15 @@
 
 	left [0] = left [1] = 0.0 ;
 	do
-	{	fraction = fp_to_double (filter_index) ;
-		indx = fp_to_int (filter_index) ;
+	{	if (data_index >= 0) /* Avoid underflow access to filter->buffer. */
+		{	fraction = fp_to_double (filter_index) ;
+			indx = fp_to_int (filter_index) ;
 
-		icoeff = filter->coeffs [indx] + fraction * (filter->coeffs [indx + 1] - filter->coeffs [indx]) ;
+			icoeff = filter->coeffs [indx] + fraction * (filter->coeffs [indx + 1] - filter->coeffs [indx]) ;
 
-		left [0] += icoeff * filter->buffer [data_index] ;
-		left [1] += icoeff * filter->buffer [data_index + 1] ;
+			left [0] += icoeff * filter->buffer [data_index] ;
+			left [1] += icoeff * filter->buffer [data_index + 1] ;
+			} ;
 
 		filter_index -= increment ;
 		data_index = data_index + 2 ;
@@ -587,15 +591,17 @@
 
 	left [0] = left [1] = left [2] = left [3] = 0.0 ;
 	do
-	{	fraction = fp_to_double (filter_index) ;
-		indx = fp_to_int (filter_index) ;
+	{	if (data_index >= 0) /* Avoid underflow access to filter->buffer. */
+		{	fraction = fp_to_double (filter_index) ;
+			indx = fp_to_int (filter_index) ;
 
-		icoeff = filter->coeffs [indx] + fraction * (filter->coeffs [indx + 1] - filter->coeffs [indx]) ;
+			icoeff = filter->coeffs [indx] + fraction * (filter->coeffs [indx + 1] - filter->coeffs [indx]) ;
 
-		left [0] += icoeff * filter->buffer [data_index] ;
-		left [1] += icoeff * filter->buffer [data_index + 1] ;
-		left [2] += icoeff * filter->buffer [data_index + 2] ;
-		left [3] += icoeff * filter->buffer [data_index + 3] ;
+			left [0] += icoeff * filter->buffer [data_index] ;
+			left [1] += icoeff * filter->buffer [data_index + 1] ;
+			left [2] += icoeff * filter->buffer [data_index + 2] ;
+			left [3] += icoeff * filter->buffer [data_index + 3] ;
+			} ;
 
 		filter_index -= increment ;
 		data_index = data_index + 4 ;
@@ -740,17 +746,19 @@
 
 	left [0] = left [1] = left [2] = left [3] = left [4] = left [5] = 0.0 ;
 	do
-	{	fraction = fp_to_double (filter_index) ;
-		indx = fp_to_int (filter_index) ;
+	{	if (data_index >= 0) /* Avoid underflow access to filter->buffer. */
+		{	fraction = fp_to_double (filter_index) ;
+			indx = fp_to_int (filter_index) ;
 
-		icoeff = filter->coeffs [indx] + fraction * (filter->coeffs [indx + 1] - filter->coeffs [indx]) ;
+			icoeff = filter->coeffs [indx] + fraction * (filter->coeffs [indx + 1] - filter->coeffs [indx]) ;
 
-		left [0] += icoeff * filter->buffer [data_index] ;
-		left [1] += icoeff * filter->buffer [data_index + 1] ;
-		left [2] += icoeff * filter->buffer [data_index + 2] ;
-		left [3] += icoeff * filter->buffer [data_index + 3] ;
-		left [4] += icoeff * filter->buffer [data_index + 4] ;
-		left [5] += icoeff * filter->buffer [data_index + 5] ;
+			left [0] += icoeff * filter->buffer [data_index] ;
+			left [1] += icoeff * filter->buffer [data_index + 1] ;
+			left [2] += icoeff * filter->buffer [data_index + 2] ;
+			left [3] += icoeff * filter->buffer [data_index + 3] ;
+			left [4] += icoeff * filter->buffer [data_index + 4] ;
+			left [5] += icoeff * filter->buffer [data_index + 5] ;
+			} ;
 
 		filter_index -= increment ;
 		data_index = data_index + 6 ;
@@ -910,48 +918,49 @@
 
 		icoeff = filter->coeffs [indx] + fraction * (filter->coeffs [indx + 1] - filter->coeffs [indx]) ;
 
-		/*
-		**	Duff's Device.
-		**	See : http://en.wikipedia.org/wiki/Duff's_device
-		*/
-		ch = channels ;
-		do
-		{
-			switch (ch % 8)
-			{	default :
-					ch -- ;
-					left [ch] += icoeff * filter->buffer [data_index + ch] ;
-					/* Falls through. */
-				case 7 :
-					ch -- ;
-					left [ch] += icoeff * filter->buffer [data_index + ch] ;
-					/* Falls through. */
-				case 6 :
-					ch -- ;
-					left [ch] += icoeff * filter->buffer [data_index + ch] ;
-					/* Falls through. */
-				case 5 :
-					ch -- ;
-					left [ch] += icoeff * filter->buffer [data_index + ch] ;
-					/* Falls through. */
-				case 4 :
-					ch -- ;
-					left [ch] += icoeff * filter->buffer [data_index + ch] ;
-					/* Falls through. */
-				case 3 :
-					ch -- ;
-					left [ch] += icoeff * filter->buffer [data_index + ch] ;
-					/* Falls through. */
-				case 2 :
-					ch -- ;
-					left [ch] += icoeff * filter->buffer [data_index + ch] ;
-					/* Falls through. */
-				case 1 :
-					ch -- ;
-					left [ch] += icoeff * filter->buffer [data_index + ch] ;
-				} ;
-			}
-		while (ch > 0) ;
+		if (data_index >= 0) /* Avoid underflow access to filter->buffer. */
+		{	/*
+			**	Duff's Device.
+			**	See : http://en.wikipedia.org/wiki/Duff's_device
+			*/
+			ch = channels ;
+			do
+			{	switch (ch % 8)
+				{	default :
+						ch -- ;
+						left [ch] += icoeff * filter->buffer [data_index + ch] ;
+						/* Falls through. */
+					case 7 :
+						ch -- ;
+						left [ch] += icoeff * filter->buffer [data_index + ch] ;
+						/* Falls through. */
+					case 6 :
+						ch -- ;
+						left [ch] += icoeff * filter->buffer [data_index + ch] ;
+						/* Falls through. */
+					case 5 :
+						ch -- ;
+						left [ch] += icoeff * filter->buffer [data_index + ch] ;
+						/* Falls through. */
+					case 4 :
+						ch -- ;
+						left [ch] += icoeff * filter->buffer [data_index + ch] ;
+						/* Falls through. */
+					case 3 :
+						ch -- ;
+						left [ch] += icoeff * filter->buffer [data_index + ch] ;
+						/* Falls through. */
+					case 2 :
+						ch -- ;
+						left [ch] += icoeff * filter->buffer [data_index + ch] ;
+						/* Falls through. */
+					case 1 :
+						ch -- ;
+						left [ch] += icoeff * filter->buffer [data_index + ch] ;
+					} ;
+				}
+			while (ch > 0) ;
+			} ;
 
 		filter_index -= increment ;
 		data_index = data_index + channels ;
--- a/tests/varispeed_test.c
+++ b/tests/varispeed_test.c
@@ -23,7 +23,7 @@
 #define fftw_cleanup()
 #endif
 
-#define	BUFFER_LEN		(1 << 15)
+#define	BUFFER_LEN		(1 << 14)
 
 static void varispeed_test (int converter, double target_snr) ;
 static void varispeed_bounds_test (int converter) ;
@@ -221,22 +221,20 @@
 	src_data.input_frames = chunk_size ;
 	src_data.output_frames = total_output_frames ;
 
-	/* Process one chunk at initial_ratio. */
-	if ((error = src_process (src_state, &src_data)))
-	{	printf ("\n\nLine %d : %s : %s\n\n", __LINE__, details, src_strerror (error)) ;
-		exit (1) ;
-		} ;
-
-	/* Now hard switch to second_ratio ... */
-	src_data.src_ratio = second_ratio ;
-	if ((error = src_process (src_state, &src_data)))
-	{	printf ("\n\nLine %d : %s : %s\n\n", __LINE__, details, src_strerror (error)) ;
-		exit (1) ;
-		} ;
-
-	/* ... and process the remaining. */
+	/* Use a max_loop_count here to enable the detection of infinite loops
+	** (due to end of input not being detected.
+	*/
 	for (k = 0 ; k < max_loop_count ; k ++)
-	{	if ((error = src_process (src_state, &src_data)) != 0)
+	{	if (k == 1)
+		{	/* Hard switch to second_ratio after processing one chunk. */
+			src_data.src_ratio = second_ratio ;
+			if ((error = src_set_ratio (src_state, second_ratio)))
+			{	printf ("\n\nLine %d : %s : %s\n\n", __LINE__, details, src_strerror (error)) ;
+				exit (1) ;
+				} ;
+			} ;
+
+		if ((error = src_process (src_state, &src_data)) != 0)
 		{	printf ("\n\nLine %d : %s : %s\n\n", __LINE__, details, src_strerror (error)) ;
 			exit (1) ;
 			} ;