< draft-ietf-codec-opus-update-09.txt		draft-ietf-codec-opus-update-10.txt >
	Network Working Group JM. Valin		Network Working Group JM. Valin
	Internet-Draft Mozilla Corporation		Internet-Draft Mozilla Corporation
	Updates: 6716 (if approved) K. Vos		Updates: 6716 (if approved) K. Vos
	Intended status: Standards Track vocTone		Intended status: Standards Track vocTone
	Expires: February 18, 2018 August 17, 2017		Expires: February 25, 2018 August 24, 2017
	Updates to the Opus Audio Codec		Updates to the Opus Audio Codec
	draft-ietf-codec-opus-update-09		draft-ietf-codec-opus-update-10
	Abstract		Abstract
	This document addresses minor issues that were found in the		This document addresses minor issues that were found in the
	specification of the Opus audio codec in RFC 6716. It updates the		specification of the Opus audio codec in RFC 6716. It updates the
	nornative decoder implementation included in the appendix of RFC		normative decoder implementation included in the appendix of RFC
	6716. The changes fixes real and potential security-related issues,		6716. The changes fixes real and potential security-related issues,
	as well minor quality-related issues.		as well minor quality-related issues.
	Status of This Memo		Status of This Memo
	This Internet-Draft is submitted in full conformance with the		This Internet-Draft is submitted in full conformance with the
	provisions of BCP 78 and BCP 79.		provisions of BCP 78 and BCP 79.
	Internet-Drafts are working documents of the Internet Engineering		Internet-Drafts are working documents of the Internet Engineering
	Task Force (IETF). Note that other groups may also distribute		Task Force (IETF). Note that other groups may also distribute
	working documents as Internet-Drafts. The list of current Internet-		working documents as Internet-Drafts. The list of current Internet-
	Drafts is at http://datatracker.ietf.org/drafts/current/.		Drafts is at http://datatracker.ietf.org/drafts/current/.
	Internet-Drafts are draft documents valid for a maximum of six months		Internet-Drafts are draft documents valid for a maximum of six months
	and may be updated, replaced, or obsoleted by other documents at any		and may be updated, replaced, or obsoleted by other documents at any
	time. It is inappropriate to use Internet-Drafts as reference		time. It is inappropriate to use Internet-Drafts as reference
	material or to cite them other than as "work in progress."		material or to cite them other than as "work in progress."
	This Internet-Draft will expire on February 18, 2018.		This Internet-Draft will expire on February 25, 2018.
	Copyright Notice		Copyright Notice
	Copyright (c) 2017 IETF Trust and the persons identified as the		Copyright (c) 2017 IETF Trust and the persons identified as the
	document authors. All rights reserved.		document authors. All rights reserved.
	This document is subject to BCP 78 and the IETF Trust's Legal		This document is subject to BCP 78 and the IETF Trust's Legal
	Provisions Relating to IETF Documents		Provisions Relating to IETF Documents
	(http://trustee.ietf.org/license-info) in effect on the date of		(http://trustee.ietf.org/license-info) in effect on the date of
	publication of this document. Please review these documents		publication of this document. Please review these documents
	skipping to change at page 4, line 49 ¶		skipping to change at page 4, line 49 ¶
	and destination regions of the memcpy() to overlap on the copy		and destination regions of the memcpy() to overlap on the copy
	from "buf" to "buf". We believe that nSamplesIn (number of input		from "buf" to "buf". We believe that nSamplesIn (number of input
	samples) is at least fs_in_khZ (sampling rate in kHz), which is		samples) is at least fs_in_khZ (sampling rate in kHz), which is
	at least 8. Since RESAMPLER_ORDER_FIR_12 is only 8, that should		at least 8. Since RESAMPLER_ORDER_FIR_12 is only 8, that should
	not be a problem once the type size is fixed.		not be a problem once the type size is fixed.
	3. The size of the buffer used RESAMPLER_MAX_BATCH_SIZE_IN, but the		3. The size of the buffer used RESAMPLER_MAX_BATCH_SIZE_IN, but the
	data stored in it was actually twice the input batch size		data stored in it was actually twice the input batch size
	(nSamplesIn<<1).		(nSamplesIn<<1).
	The allocated buffers involved (buf and S->sFIR) are actually larger
	than they need to be for the batch size used, so no out-of-bounds
	read or write is possible. Therefore the bug cannot be exploited.
	The code can be fixed by applying the following changes to line 78 of		The code can be fixed by applying the following changes to line 78 of
	silk/resampler_private_IIR_FIR.c:		silk/resampler_private_IIR_FIR.c:
	<CODE BEGINS>		<CODE BEGINS>
	)		)
	{		{
	silk_resampler_state_struct *S = \		silk_resampler_state_struct *S = \
	(silk_resampler_state_struct *)SS;		(silk_resampler_state_struct *)SS;
	opus_int32 nSamplesIn;		opus_int32 nSamplesIn;
	opus_int32 max_index_Q16, index_increment_Q16;		opus_int32 max_index_Q16, index_increment_Q16;
	skipping to change at page 6, line 4 ¶		skipping to change at page 5, line 48 ¶
	/* More iterations to do; copy last part of \		/* More iterations to do; copy last part of \
	filtered signal to beginning of buffer */		filtered signal to beginning of buffer */
	- silk_memcpy( buf, &buf[ nSamplesIn << 1 ], \		- silk_memcpy( buf, &buf[ nSamplesIn << 1 ], \
	RESAMPLER_ORDER_FIR_12 * sizeof( opus_int32 ) );		RESAMPLER_ORDER_FIR_12 * sizeof( opus_int32 ) );
	+ silk_memmove( buf, &buf[ nSamplesIn << 1 ], \		+ silk_memmove( buf, &buf[ nSamplesIn << 1 ], \
	RESAMPLER_ORDER_FIR_12 * sizeof( opus_int16 ) );		RESAMPLER_ORDER_FIR_12 * sizeof( opus_int16 ) );
	} else {		} else {
	break;		break;
	}		}
	}		}
	/* Copy last part of filtered signal to the state for \		/* Copy last part of filtered signal to the state for \
	the next call */		the next call */
	- silk_memcpy( S->sFIR, &buf[ nSamplesIn << 1 ], \		- silk_memcpy( S->sFIR, &buf[ nSamplesIn << 1 ], \
	RESAMPLER_ORDER_FIR_12 * sizeof( opus_int32 ) );		RESAMPLER_ORDER_FIR_12 * sizeof( opus_int32 ) );
	+ silk_memcpy( S->sFIR, &buf[ nSamplesIn << 1 ], \		+ silk_memcpy( S->sFIR, &buf[ nSamplesIn << 1 ], \
	RESAMPLER_ORDER_FIR_12 * sizeof( opus_int16 ) );		RESAMPLER_ORDER_FIR_12 * sizeof( opus_int16 ) );
	}		}
	<CODE ENDS>		<CODE ENDS>
	6. Integer wrap-around in inverse gain computation		6. Integer wrap-around in inverse gain computation
	It was discovered through decoder fuzzing that some bitstreams could		It was discovered through decoder fuzzing that some bitstreams could
	produce integer values exceeding 32-bits in		produce integer values exceeding 32-bits in
	LPC_inverse_pred_gain_QA(), causing a wrap-around. Although the		LPC_inverse_pred_gain_QA(), causing a wrap-around. The C standard
	authors are not aware of any way to exploit the bug, the C standard		considers this behavior as undefined. The following patch to line 87
	considers the behavior as undefined. The following patch to line 87
	of silk/LPC_inv_pred_gain.c detects values that do not fit in a		of silk/LPC_inv_pred_gain.c detects values that do not fit in a
	32-bit integer and considers the corresponding filters unstable:		32-bit integer and considers the corresponding filters unstable:
	<CODE BEGINS>		<CODE BEGINS>
	/* Update AR coefficient */		/* Update AR coefficient */
	for( n = 0; n < k; n++ ) {		for( n = 0; n < k; n++ ) {
	- tmp_QA = Aold_QA[ n ] - MUL32_FRAC_Q( \		- tmp_QA = Aold_QA[ n ] - MUL32_FRAC_Q( \
	Aold_QA[ k - n - 1 ], rc_Q31, 31 );		Aold_QA[ k - n - 1 ], rc_Q31, 31 );
	- Anew_QA[ n ] = MUL32_FRAC_Q( tmp_QA, rc_mult2 , mult2Q );		- Anew_QA[ n ] = MUL32_FRAC_Q( tmp_QA, rc_mult2 , mult2Q );
	+ opus_int64 tmp64;		+ opus_int64 tmp64;
	skipping to change at page 10, line 50 ¶		skipping to change at page 10, line 50 ¶
	98e09bbafed329e341c3b4052e9c4ba5fc83f9b1 testvector12m.dec		98e09bbafed329e341c3b4052e9c4ba5fc83f9b1 testvector12m.dec
	Note that the decoder input bitstream files (.bit) are unchanged.		Note that the decoder input bitstream files (.bit) are unchanged.
	12. Security Considerations		12. Security Considerations
	This document fixes two security issues reported on Opus and that		This document fixes two security issues reported on Opus and that
	affect the reference implementation in RFC 6716 [RFC6716]: CVE-		affect the reference implementation in RFC 6716 [RFC6716]: CVE-
	2013-0899 <https://nvd.nist.gov/vuln/detail/CVE-2013-0899> and CVE-		2013-0899 <https://nvd.nist.gov/vuln/detail/CVE-2013-0899> and CVE-
	2017-0381 <https://nvd.nist.gov/vuln/detail/CVE-2017-0381>. CVE-		2017-0381 <https://nvd.nist.gov/vuln/detail/CVE-2017-0381>. CVE-
	2013-0899 is fixed by Section 4 and could theoretically cause an		2013-0899 theoretically could have caused an information leak. The
	information leak, but the leaked information would at the very least		leaked information would have gone through the decoder process before
	go through the decoder process before being accessible to the		being accessible to the attacker. It is fixed by Section 4. CVE-
	attacker. Also, the bug can only be triggered by Opus packets at		2017-0381 could have resulted in a 16-bit out-of-bounds read from a
	least 24 MB in size. CVE-2017-0381 is fixed by Section 7 and can		fixed location. It is fixed in Section 7. Beyond the two fixed
	only result in a 16-bit out-of-bounds read to a fixed location 256		CVEs, this document adds no new security considerations on top of RFC
	bytes before a constant table. That location would normally be part		6716 [RFC6716].
	of an executable's read-only data segment, but if that is not the
	case, the bug could at worst results in either a crash or the leakage
	of 16 bits of information from that fixed memory location (if the
	attacker has access to the decoded output). Despite the claims of
	the CVE, the bug cannot results in arbitrary code execution. Beyond
	the two fixed CVEs, this document adds no new security considerations
	on top of RFC 6716 [RFC6716].
	13. IANA Considerations		13. IANA Considerations
	This document makes no request of IANA.		This document makes no request of IANA.
	Note to RFC Editor: this section may be removed on publication as an		Note to RFC Editor: this section may be removed on publication as an
	RFC.		RFC.
	14. Acknowledgements		14. Acknowledgements
End of changes. 8 change blocks.
	25 lines changed or deleted		14 lines changed or added
This html diff was produced by rfcdiff 1.48. The latest version is available from http://tools.ietf.org/tools/rfcdiff/