bug#40832: alsa-lib cannot find its plugins

[Danny Milosavljevic]

Hi Leo,

On Tue, 28 Jul 2020 19:56:23 -0400
Leo Famulari <leo@famulari.name> wrote:

> On Tue, Jul 28, 2020 at 12:52:41PM +0200, Danny Milosavljevic wrote:
> > some comments on the lastest patch:  
> 
> Thank you for reviewing the patch!
> 
> > * The entire alsa-lib seems to use the idiom "malloc and then strcpy", or
> > "malloc and then sprintf", or, worse, "malloc, strcpy and multiple strcat".
> > These are a buffer overflow waiting to happen (when changing part of those
> > while doing ongoing maintenance;  also the places where they use "+" is not
> > checked for overflow).  That said, if they do it, we can do it that way, 
> > too.  
> 
> This confirms what I felt — it's hard to feel confident about the bounds
> checking in this code. It seems to be based on the names of the plugin
> libraries not exceeding some magic length. It's hard to balance "doing
> the right thing" and using upstream's idioms.

After thinking about it more, I think it's much worse if the thing that is
stuck into the malloced block is the value of an environment variable.

When it's a compile-time variable you basically trust the code and the
distribution package not to have too-long paths in there that could overflow
the "+" in malloc(... + ).  A distribution or upstream could do much worse
things than that, so that is not a credible threat to worry about.

For a runtime variable like the environment variable (that anyone can set to
anything), I am very much in favor of not using malloc(... + ) and instead
using asprintf, in order to prevent an exploitable buffer overflow just by
setting up an environment variable.

> When writing the patch, my investigation into the code made decide that
> it would not overflow, but now I don't remember why I thought that.

Thanks for that remark.  It made me think and I came to the recommendation
above.

pgp3CVad0DRl4.pgp
Description: OpenPGP digital signature