CVE-2010-2089: Issue 7673: audioop: check that length is a multiple of the size

Created on 2010-01-11 01:05 by vstinner, last changed 2022-04-11 14:56 by admin. This issue is now closed.

Files

File name

Uploaded

Description

Edit

audioop_check_length-2.patch

vstinner, 2010-07-01 01:55

Messages (10)

msg97566 - (view)

Author: STINNER Victor (vstinner) *

Date: 2010-01-11 01:05

Most functions of audioop takes as input a byte string (audio data) and a size argument (number of bytes of a sample). Functions don’t check that the byte string length is a multiple of the size. It leads to read and write from/to uninitialised memory and might crash.

Example on writing into uninitilized memory:

$ python -c "import audioop; audioop.reverse('X', 2)"
Fatal Python error: Inconsistent interned string state.
Abandon

It allocates a string of 1 byte and write 2 bytes into this string => memory corruption.

Attached patch creates audioop_check_size() and audioop_check_parameters() functions.

msg108733 - (view)

Author: Arfrever Frehtes Taifersar Arahesis (Arfrever) *

Date: 2010-06-26 16:32

http://web.nvd.nist.gov/view/vuln/detail?vulnId=CVE-2010-2089 claims that this issue is about security vulnerability. This problem seems to also affect at least Python 2.6.

msg108933 - (view)

Author: Mark Dickinson (mark.dickinson) *

Date: 2010-06-29 19:02

The patch looks fine to me.

• Please could you add some tests, to exercise the ‘not a whole number of frames’ errors?

• The patch obviously predates the grand reindenting, so its indentation needs fixing up

PEP 7 nits:

• Please don’t put spaces just inside the parens in an ‘if’ statement: i.e., use "if (size != 1 …)", not "if ( size != 1 …)" (I notice that the "if ( x == NULL )" style is already prevalent, though not universal, in the module, though.)

• the ‘else’ clause of an if should be at the start of the line (i.e., on a new line below the closing brace of the 'if’, if present)

Is there any particular reason that Python 3.1 is not included in the versions?

msg109027 - (view)

Author: STINNER Victor (vstinner) *

Date: 2010-07-01 01:55

@Mark: Here is the updated version of the patch including all of your remarks. I fixed 3 bugs in my patch: the checks of adpcm2lin(), alaw2lin() and audioop.ulaw2lin() were incomplete (len was not checked).

I added 3.1 to the version field.

msg109171 - (view)

Author: Mark Dickinson (mark.dickinson) *

Date: 2010-07-03 09:56

The new patch looks fine to me.

This is rather last minute for 2.7, and I’m very uncomfortable committing anything substantial this close to the release. Still, if it’s really a security vulnerability then it would be good to get it in.

For what it’s worth, the code looks fine to me, and I’ve tested thoroughly; I can’t see any reasons this could cause problems.

Raising priority to release blocker just to alert Benjamin to the issue, and get his permission to go ahead (or not) before the release.

msg109172 - (view)

Author: Antoine Pitrou (pitrou) *

Date: 2010-07-03 10:24

The following error messages looks strange to me:

• if (((len / size) & 1) != 0) {

•    PyErr\_SetString(AudioopError, "not a whole number of frames");
  

•    return NULL;
  

• }

Perhaps you meant "not an even number of frames"?

msg109173 - (view)

Author: Mark Dickinson (mark.dickinson) *

Date: 2010-07-03 10:36

Well, that would depend on how you define a 'frame’, I guess.

msg109183 - (view)

Author: STINNER Victor (vstinner) *

Date: 2010-07-03 13:47

This issue is a security vulnerability referenced as CVE-2010-2089.

Fixed in 2.7 (r82492), 2.6 (r82494), 3.2 (r82495) and 3.1 (r82496).

–

Perhaps you meant "not an even number of frames"?

Hum, no: the input data is a stereo sound track. A “frame” includes left and right channels.

msg109211 - (view)

Author: Antoine Pitrou (pitrou) *

Date: 2010-07-04 09:19

It seems you introduced a reference leak, Victor. http://mail.python.org/pipermail/python-checkins/2010-July/094756.html

msg109212 - (view)

Author: Mark Dickinson (mark.dickinson) *

Date: 2010-07-04 10:17

Fixed in r82527 (py3k), r82528 (release31-maint).

History

Date

User

Action

Args

2022-04-11 14:56:56

admin

set

github: 51922

2021-11-04 14:12:54

eryksun

set

nosy: - ahmedsayeed1982

2021-11-04 14:12:47

eryksun

set

messages: - msg405688

2021-11-04 12:08:35

ahmedsayeed1982

set

nosy: + ahmedsayeed1982, - mark.dickinson, pitrou, vstinner, benjamin.peterson, Arfrever
messages: + msg405688

2010-07-04 10:17:04

mark.dickinson

set

status: open -> closed

messages: + msg109212

2010-07-04 09:19:44

pitrou

set

status: closed -> open
priority: release blocker -> high
resolution: fixed -> accepted
messages: + msg109211

2010-07-03 13:48:15

vstinner

set

status: open -> closed
resolution: fixed

2010-07-03 13:47:50

vstinner

set

assignee: mark.dickinson -> vstinner
messages: + msg109183

2010-07-03 12:27:24

vstinner

set

files: - audioop_check_length.patch

2010-07-03 10:36:17

mark.dickinson

set

messages: + msg109173

2010-07-03 10:24:48

pitrou

set

nosy: + pitrou
messages: + msg109172

2010-07-03 10:11:23

mark.dickinson

set

stage: commit review

2010-07-03 09:57:49

mark.dickinson

set

type: crash -> security

2010-07-03 09:56:33

mark.dickinson

set

priority: normal -> release blocker

nosy: + benjamin.peterson
messages: + msg109171

assignee: mark.dickinson

2010-07-01 01:55:37

vstinner

set

files: + audioop_check_length-2.patch

messages: + msg109027
versions: + Python 3.1

2010-06-29 19:02:08

mark.dickinson

set

nosy: + mark.dickinson
messages: + msg108933

2010-06-26 16:32:10

Arfrever

set

messages: + msg108733
versions: + Python 2.6

2010-06-25 22:06:45

Arfrever

set

nosy: + Arfrever

2010-01-11 01:05:55

vstinner

create