[PATCH] Fix possible sprintf overrun in snd_pcm_hw_open

[PATCH] Fix possible sprintf overrun in snd_pcm_hw_open

[David Henningsson]

[-- Attachment #1: Type: text/plain, Size: 274 bytes --]

I spotted this while reading code a few weeks ago, and I ran it through 
the Ubuntu security team just to be sure.
They decided it was not needing any security embargo or similar, so here 
comes the patch.


-- 
David Henningsson, Canonical Ltd.
http://launchpad.net/~diwic

[-- Attachment #2: 0001-Fix-possible-sprintf-overrun-in-snd_pcm_hw_open.patch --]
[-- Type: text/x-patch, Size: 970 bytes --]

>From 3333d9bb8d8f9cc95f9dbf68d0a703a4e832a948 Mon Sep 17 00:00:00 2001
From: David Henningsson <david.henningsson@canonical.com>
Date: Wed, 8 Dec 2010 11:06:59 +0100
Subject: [PATCH] Fix possible sprintf overrun in snd_pcm_hw_open

BugLink: http://launchpad.net/bugs/668487

Possible buffer overrun if the number of "card" and "device"
are absurdly high, especially on 64-bit platforms.

Signed-off-by: David Henningsson <david.henningsson@canonical.com>
---
 src/pcm/pcm_hw.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/src/pcm/pcm_hw.c b/src/pcm/pcm_hw.c
index 9d243d5..ce74ad4 100644
--- a/src/pcm/pcm_hw.c
+++ b/src/pcm/pcm_hw.c
@@ -1270,7 +1270,7 @@ int snd_pcm_hw_open(snd_pcm_t **pcmp, const char *name,
 		SNDERR("invalid stream %d", stream);
 		return -EINVAL;
 	}
-	sprintf(filename, filefmt, card, device);
+	snprintf(filename, sizeof(filename), filefmt, card, device);
 
       __again:
       	if (attempt++ > 3) {
-- 
1.7.1


[-- Attachment #3: Type: text/plain, Size: 160 bytes --]

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH] Fix possible sprintf overrun in snd_pcm_hw_open

[Clemens Ladisch]

David Henningsson wrote:
> Possible buffer overrun if the number of "card" and "device"
> are absurdly high, especially on 64-bit platforms.

The size of "int" is 32 bits even on 64-bit platforms.

As far as I can see, there is no bug.


Regards,
Clemens

Re: [PATCH] Fix possible sprintf overrun in snd_pcm_hw_open

[David Henningsson]

On 2010-12-08 13:12, Clemens Ladisch wrote:
> David Henningsson wrote:
>> Possible buffer overrun if the number of "card" and "device"
>> are absurdly high, especially on 64-bit platforms.
>
> The size of "int" is 32 bits even on 64-bit platforms.

Seems you're right, then I learned something new today :-)

Although this might be compiler dependent, and some exotic platform 
might decide otherwise in the future?

> As far as I can see, there is no bug.

Even for 32-bit platforms, you would still overrun the buffer if you set 
card = device = −2147483647.

-- 
David Henningsson, Canonical Ltd.
http://launchpad.net/~diwic
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH] Fix possible sprintf overrun in snd_pcm_hw_open

[David Henningsson]

On 2010-12-08 13:56, David Henningsson wrote:
> On 2010-12-08 13:12, Clemens Ladisch wrote:
>> David Henningsson wrote:
>>> Possible buffer overrun if the number of "card" and "device"
>>> are absurdly high, especially on 64-bit platforms.
>>
>> The size of "int" is 32 bits even on 64-bit platforms.
>
> Seems you're right, then I learned something new today :-)
>
> Although this might be compiler dependent, and some exotic platform
> might decide otherwise in the future?
>
>> As far as I can see, there is no bug.
>
> Even for 32-bit platforms, you would still overrun the buffer if you set
> card = device = −2147483647.
>

...or maybe not, forgot that the %d characters are removed. Oh well, 
doesn't hurt to change it into snprintf anyway ;-)

-- 
David Henningsson, Canonical Ltd.
http://launchpad.net/~diwic
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel