Re: mass bug filing for undefined sn?printf use

[Ryan Niebur]

Hi,

On Fri, Jan 16, 2009 at 10:29:18AM +, peter green wrote:
> >IMHO any bugs filed merely due to the presence of the code without the
> > means to trigger the error in normal builds should be wishlist.
> What is particularlly insiduous about this issue is that it could  
> easilly be activated by accident if the maintainer or a NMUer builds and  
> uploads a new version of the package on a system/chroot that happens to  
> have hardening-wrapper installed (most likely left over from building a  
> previous package).
>

hardening-wrapper doesn't do anything unless it has
"DEB_BUILD_HARDENING=1" in it's environment or in
/etc/hardening-wrapper.conf (which does not exist by default)

> IMO because it can lead to packages that were not previously broken  
> breaking after a rebuild this deserves a severity of at least normal
>

-- 
_
Ryan Niebur
ryanrya...@gmail.com


signature.asc
Description: Digital signature

Re: mass bug filing for undefined sn?printf use

[peter green]

>IMHO any bugs filed merely due to the presence of the code without the
> means to trigger the error in normal builds should be wishlist.
What is particularlly insiduous about this issue is that it could 
easilly be activated by accident if the maintainer or a NMUer builds and 
uploads a new version of the package on a system/chroot that happens to 
have hardening-wrapper installed (most likely left over from building a 
previous package).


IMO because it can lead to packages that were not previously broken 
breaking after a rebuild this deserves a severity of at least normal



--
To UNSUBSCRIBE, email to debian-devel-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org

Re: mass bug filing for undefined sn?printf use

[Joost Yervante Damad]

> Joost Yervante Damad 
>timidity

Fixed locally, but quite intrusive, will need some more time; also will be 
combined with other fixes.

Joost


-- 
To UNSUBSCRIBE, email to debian-devel-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org

Re: mass bug filing for undefined sn?printf use

[Michael Tautschnig]

[...]
> Michael Tautschnig 
>binutils-h8300-hms
> 
[...]

Fixed in unstable.

Best,
Michael



pgpiPwdmopCV1.pgp
Description: PGP signature

Re: mass bug filing for undefined sn?printf use

[Julien Cristau]

On Sun, Dec 28, 2008 at 09:53:40 +0100, Adeodato Simó wrote:

> Debian X Strike Force 
>libx11
> 
Fixed upstream and in experimental.
(http://bugs.freedesktop.org/show_bug.cgi?id=14898)

Cheers,
Julien


-- 
To UNSUBSCRIBE, email to debian-devel-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org

Re: mass bug filing for undefined sn?printf use

[George Danchev]

On Friday 02 January 2009 05:04:08 Russ Allbery wrote:
> "Paul Wise"  writes:
> > On Fri, Jan 2, 2009 at 3:50 AM, Kees Cook  wrote:
> >> Oh!  Good catch, thank you.  I've started a re-run with the regex
> >> changed.  So far, it's already caught new stuff.  I'll post updated
> >> details once it has finished.
> >
> > Could this test be added to lintian?
>
> The thread so far seems to indicate the false positive rate isn't great.
> People usually find Lintian checks with a lot of false positives rather
> annoying.  It can be worth it if the problem is sufficiently severe, but
> it always makes me nervous to add.

FYI: such a check will be added to cppcheck too.

-- 
pub 4096R/0E4BD0AB 2003-03-18 


-- 
To UNSUBSCRIBE, email to debian-devel-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org

Re: mass bug filing for undefined sn?printf use

[Russ Allbery]

"Paul Wise"  writes:
> On Fri, Jan 2, 2009 at 3:50 AM, Kees Cook  wrote:

>> Oh!  Good catch, thank you.  I've started a re-run with the regex
>> changed.  So far, it's already caught new stuff.  I'll post updated
>> details once it has finished.

> Could this test be added to lintian?

The thread so far seems to indicate the false positive rate isn't great.
People usually find Lintian checks with a lot of false positives rather
annoying.  It can be worth it if the problem is sufficiently severe, but
it always makes me nervous to add.

We could possibly add an experimental tag, though, to get an idea of what
the false positive rate looks like.  We're trying that with a few other
ones at the moment.

-- 
Russ Allbery (r...@debian.org)   


-- 
To UNSUBSCRIBE, email to debian-devel-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org

Re: mass bug filing for undefined sn?printf use

[Paul Wise]

On Fri, Jan 2, 2009 at 3:50 AM, Kees Cook  wrote:

> Oh!  Good catch, thank you.  I've started a re-run with the regex changed.
> So far, it's already caught new stuff.  I'll post updated details once it
> has finished.

Could this test be added to lintian?

-- 
bye,
pabs

http://wiki.debian.org/PaulWise


-- 
To UNSUBSCRIBE, email to debian-devel-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org

Re: mass bug filing for undefined sn?printf use

[Kees Cook]

On Wed, Dec 31, 2008 at 07:01:44PM -0800, Nicholas Breen wrote:
> While fixing one of the affected packages, I discovered that it was
> using similarly problematic syntax to act as a strcat replacement of the
> form 'sprintf(buf, "%s\n", buf)', which that regexp didn't catch.  I
> can't imagine that's a common mistake, but it's easy enough to match on
> as well:
> 
>   pcregrep -M 'sprintf\s*\(\s*([^,]*)\s*,\s*"%s[^"]*"\s*,\s*\1\s*[,)]'

Oh!  Good catch, thank you.  I've started a re-run with the regex changed.
So far, it's already caught new stuff.  I'll post updated details once it
has finished.

-- 
Kees Cook@debian.org


-- 
To UNSUBSCRIBE, email to debian-devel-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org

Re: mass bug filing for undefined sn?printf use

[Nicholas Breen]

On Sun, Dec 28, 2008 at 12:42:46AM -0800, Kees Cook wrote:
> Hi,
> 
> I'd like to seek advice before I perform a mass-bug filing for this
> unstable (though semi-common) use of "sprintf" and "snprintf":
[...]
>   pcregrep -M 'sprintf\s*\(\s*([^,]*)\s*,\s*"%s[^"]*"\s*,\s*\1\s*,'

While fixing one of the affected packages, I discovered that it was
using similarly problematic syntax to act as a strcat replacement of the
form 'sprintf(buf, "%s\n", buf)', which that regexp didn't catch.  I
can't imagine that's a common mistake, but it's easy enough to match on
as well:

  pcregrep -M 'sprintf\s*\(\s*([^,]*)\s*,\s*"%s[^"]*"\s*,\s*\1\s*[,)]'

> gabedit
> gromacs
> openbabel

All pending upload, thanks.


-- 
Nicholas Breen
nbr...@ofb.net


-- 
To UNSUBSCRIBE, email to debian-devel-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org

Re: mass bug filing for undefined sn?printf use

[Aurelien Jarno]

On Sun, Dec 28, 2008 at 09:53:40AM +0100, Adeodato Simó wrote:
> > Attached is a list of affected packages,
> 
> Piping through dd-list(1) gives:
> Aurelien Jarno 
>med-fichier
>sdlperl (U)
> 

sdlperl is fixed in both unstable and experimental.

-- 
  .''`.  Aurelien Jarno | GPG: 1024D/F1BCDB73
 : :' :  Debian developer   | Electrical Engineer
 `. `'   aure...@debian.org | aurel...@aurel32.net
   `-people.debian.org/~aurel32 | www.aurel32.net


-- 
To UNSUBSCRIBE, email to debian-devel-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org

Re: mass bug filing for undefined sn?printf use

[Kees Cook]

On Tue, Dec 30, 2008 at 08:03:13PM +0100, Arthur de Jong wrote:
> I've just performed a test with the following code on my system (sid,
> hardening-wrapper not installed, compiled with gcc without any extra
> flags):
> 
>   char buf[20];
>   strcpy(buf,"FOO");
>   snprintf(buf,sizeof(buf),"%s%s",buf,"BAR");
>   printf("%s\n",buf);
>   strcpy(buf,"BAR");
>   snprintf(buf,sizeof(buf),"%s%s","FOO",buf);
>   printf("%s\n",buf);
> 
> which returned
> 
> BAR
> FOOFOO

Changing your code to "sprintf" (since snprintf unfortunately tends to be
in the minority still), the output for the first changes to "FOOBAR".

-- 
Kees Cook@debian.org


-- 
To UNSUBSCRIBE, email to debian-devel-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org

Re: mass bug filing for undefined sn?printf use

[Reinhard Tartler]

Evgeni Golov  writes:

> On Sun, 28 Dec 2008 09:53:40 +0100 Adeodato Simó wrote:
>
>> Evgeni Golov 
>>desmume (U)
>
> Forwarded upstream, they'll fix that asap.

thanks for taking care for this!

-- 
Gruesse/greetings,
Reinhard Tartler, KeyID 945348A4


-- 
To UNSUBSCRIBE, email to debian-devel-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org

Re: mass bug filing for undefined sn?printf use

[Arthur de Jong]

On Tue, 2008-12-30 at 10:41 -0600, Steve Langasek wrote:
> On Tue, Dec 30, 2008 at 10:06:41AM +0100, Arthur de Jong wrote:
> > On Sun, 2008-12-28 at 12:02 -0600, Steve Langasek wrote:
> > > I don't know whether these are also a problem in practice - but if so,
> > > using sprintf(buf + strlen(buf) [...]) is definitely wrong.
> 
> > I don't know if any of my code uses such a construct but why is that
> > wrong as long as [...] doesn't contain buf?
> 
> That's not the context of this discussion; we were talking about buggy code
> that *did* use buf as one of the args to the format string.

Ok, I misunderstood. Thanks.

In that case there can be a great number of situations in which the
buffer may be filled with it's own content (eg. pass the same array as
two arguments to a function and use the above construct in that
function, assignments to temporary variables, etc, etc). Very hard to
check for.

Perhaps this would be a good test for tools such as split, rats or
flawfinder (none of them currently warn about this problem and neither
do any gcc flags that I commonly use). Those kind of tools already to
quite some analysis of source code so perhaps it isn't too difficult to
implement it there.

I've just performed a test with the following code on my system (sid,
hardening-wrapper not installed, compiled with gcc without any extra
flags):

  char buf[20];
  strcpy(buf,"FOO");
  snprintf(buf,sizeof(buf),"%s%s",buf,"BAR");
  printf("%s\n",buf);
  strcpy(buf,"BAR");
  snprintf(buf,sizeof(buf),"%s%s","FOO",buf);
  printf("%s\n",buf);

which returned

BAR
FOOFOO

so in any case the behaviour is not as could be naively expected
(FOOBAR).

-- 
-- arthur - adej...@debian.org - http://people.debian.org/~adejong --


signature.asc
Description: This is a digitally signed message part

Re: mass bug filing for undefined sn?printf use

[Steve Langasek]

On Tue, Dec 30, 2008 at 10:06:41AM +0100, Arthur de Jong wrote:
> On Sun, 2008-12-28 at 12:02 -0600, Steve Langasek wrote:
> > I don't know whether these are also a problem in practice - but if so,
> > using sprintf(buf + strlen(buf) [...]) is definitely wrong.

> I don't know if any of my code uses such a construct but why is that
> wrong as long as [...] doesn't contain buf?

That's not the context of this discussion; we were talking about buggy code
that *did* use buf as one of the args to the format string.

-- 
Steve Langasek   Give me a lever long enough and a Free OS
Debian Developer   to set it on, and I can move the world.
Ubuntu Developerhttp://www.debian.org/
slanga...@ubuntu.com vor...@debian.org


-- 
To UNSUBSCRIBE, email to debian-devel-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org

Re: mass bug filing for undefined sn?printf use

[Arthur de Jong]

On Sun, 2008-12-28 at 12:02 -0600, Steve Langasek wrote:
> I don't know whether these are also a problem in practice - but if so,
> using sprintf(buf + strlen(buf) [...]) is definitely wrong.

I don't know if any of my code uses such a construct but why is that
wrong as long as [...] doesn't contain buf? (assuming proper bound
checks are done and other parameters are sane)

Thanks.

-- 
-- arthur - adej...@debian.org - http://people.debian.org/~adejong --


signature.asc
Description: This is a digitally signed message part

Re: mass bug filing for undefined sn?printf use

[Thomas Viehmann]

Hi,

Kees Cook wrote:
> On Sun, Dec 28, 2008 at 03:10:37PM +0100, Thomas Viehmann wrote:
>> How about either matching stuff against the build logs or recompiling

> I didn't have the resources to do this, but it's be great if someone could.

If you have the means of recompiling, say with pbuilder, that should
give you logs to look at.

>> with a compiler that actually fails when asked to compile a file that
>> matches? That would seem to have potential for reducing the number of
>> false positives.
> 
> I'd really love that too -- I just don't know how to modify the compiler to
> do it.  :)

You could try to use a wrapper for the various gcc binaries that greps
through the *.c?? it is passed with your regexp, logging the matches and
then calling the real binary. But then maybe I just don't have a clue
how to do it better.
It'll still have false positives from the regexp itself, but you'll
exclude code that isn't used.

Kind regards

T.
-- 
Thomas Viehmann, http://thomas.viehmann.net/


-- 
To UNSUBSCRIBE, email to debian-devel-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org

Re: mass bug filing for undefined sn?printf use

[LI Daobing (李道兵)]

On Sun, Dec 28, 2008 at 4:53 PM, Adeodato Simó  wrote:
>> Attached is a list of affected packages,
>
> Piping through dd-list(1) gives:
>
> LI Daobing 
>   liblunar
forwarded to upstream, and he will fix it in next release.

>   openbabel (U)
left to debichem team.



-- 
Best Regards,
 LI Daobing


--
To UNSUBSCRIBE, email to debian-devel-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org

Re: mass bug filing for undefined sn?printf use

[Asheesh Laroia]

On Mon, 29 Dec 2008, Kees Cook wrote:


Hi,

On Sun, Dec 28, 2008 at 03:10:37PM +0100, Thomas Viehmann wrote:

How about either matching stuff against the build logs or recompiling


I didn't have the resources to do this, but it's be great if someone could.


I'll work on this now.

-- Asheesh.

--
Your ignorance cramps my conversation.


--
To UNSUBSCRIBE, email to debian-devel-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org

Re: mass bug filing for undefined sn?printf use

[Kees Cook]

On Sun, Dec 28, 2008 at 10:27:16AM +, Neil Williams wrote:
> On Sun, 28 Dec 2008 00:42:46 -0800 Kees Cook  wrote:
> > In Debian, some tools already compile natively with -D_FORTIFY_SOURCE=2,
> > and some have Build-Depends on "hardening-wrapper", which enables this
> > compiler flag.  As such, it seems sensible to have all affected packages
> > fixed since the results of such a call could change.  (Though it is not an
> > RC issue.)
> 
> By all affected packages, do you mean packages that use the code or
> packages that use the code *AND* compile with  or
> Build-Depend on hardening-wrapper?
> 
> IMHO any bugs filed merely due to the presence of the code without the
> means to trigger the error in normal builds should be wishlist.

Sorry for the confusion -- I meant "present in the code", not "actively
broken".  I agree it's not a "normal" bug, but I'd like to see the bug at
least as "low" since (with a stock glibc) the bug would appear if a
maintainer decided to use "hardening-wrapper".

> > Thoughts?
> 
> Split the list according to packages that merely match the regexp and
> those that match the regexp *AND* match a second regexp indicating that
> the build system either uses -D_FORTIFY_SOURCE=2 or hardening-wrapper?

Good idea, those can be opened with "normal" severity.

-Kees

-- 
Kees Cook@debian.org


-- 
To UNSUBSCRIBE, email to debian-devel-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org

Re: mass bug filing for undefined sn?printf use

[Kees Cook]

Hi,

On Sun, Dec 28, 2008 at 03:10:37PM +0100, Thomas Viehmann wrote:
> How about either matching stuff against the build logs or recompiling

I didn't have the resources to do this, but it's be great if someone could.

> with a compiler that actually fails when asked to compile a file that
> matches? That would seem to have potential for reducing the number of
> false positives.

I'd really love that too -- I just don't know how to modify the compiler to
do it.  :)

-Kees

-- 
Kees Cook@debian.org


-- 
To UNSUBSCRIBE, email to debian-devel-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org

Re: mass bug filing for undefined sn?printf use

[Kees Cook]

On Sun, Dec 28, 2008 at 01:51:45PM -0600, Steve Langasek wrote:
> On Sun, Dec 28, 2008 at 12:42:46AM -0800, Kees Cook wrote:
> > samba
> 
> Another false positive, AFAICS:
> 
> $   pcregrep -rM 'sprintf\s*\(\s*([^,]*)\s*,\s*"%s[^"]*"\s*,\s*\1\s*,' source
> source/libads/kerberos.c: fname = talloc_asprintf(dname, 
> "%s/krb5.conf.%s", dname, domain);

Thanks, I've marked samba and wmi as false alarms.

> Perhaps adding a \b to the front of the regexp would be appropriate?

I didn't include a word-break intentionally; I think the benefits are
greater, since it catches luckily-named variations like g_sprintf (which
I knew of ahead of time) and ircsprintf (found during search).

-Kees

-- 
Kees Cook@debian.org


-- 
To UNSUBSCRIBE, email to debian-devel-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org

Re: mass bug filing for undefined sn?printf use

[Evgeni Golov]

On Sun, 28 Dec 2008 09:53:40 +0100 Adeodato Simó wrote:

> Evgeni Golov 
>desmume (U)

Forwarded upstream, they'll fix that asap.


--
To UNSUBSCRIBE, email to debian-devel-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org

Re: mass bug filing for undefined sn?printf use

[Adam Borowski]

On Sun, Dec 28, 2008 at 12:02:46PM -0600, Steve Langasek wrote:
> On Sun, Dec 28, 2008 at 12:42:46AM -0800, Kees Cook wrote:
> >   pcregrep -M 'sprintf\s*\(\s*([^,]*)\s*,\s*"%s[^"]*"\s*,\s*\1\s*,'
> >   pcregrep -M 'snprintf\s*\(\s*([^,]*)\s*,[^,]*,\s*"%s[^"]*"\s*,\s*\1\s*,'
> 
> I would note that this regexp, and the proposed solution, will not match
> i18nized format strings; i.e.,
> 
>   sprintf(buf, _("%s plus %d"), buf, k);

If _any_ of the translations doesn't start with %s, it will break.  Oh, and
you used sprintf() not snprintf() -- it's a guaranteed trample&segfault
here.  From what I've seen, many languages like to quote things not usually
quoted in English, so the core will be filled with '`', '“' or '»'.

The sprintf(buf, "%s foo", buf) hack is indeed something that should be
rooted out.  It happens to work on glibc (usually), but it's neither
portable nor sane.

> I don't know whether these are also a problem in practice - but if so, using
> sprintf(buf + strlen(buf) [...]) is definitely wrong.

In that case, I see no choice but using a second buffer...

-- 
1KB // Microsoft corollary to Hanlon's razor:
//  Never attribute to stupidity what can be
//  adequately explained by malice.


-- 
To UNSUBSCRIBE, email to debian-devel-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org

Re: mass bug filing for undefined sn?printf use

[Steve Langasek]

On Sun, Dec 28, 2008 at 12:42:46AM -0800, Kees Cook wrote:
> samba

Another false positive, AFAICS:

$   pcregrep -rM 'sprintf\s*\(\s*([^,]*)\s*,\s*"%s[^"]*"\s*,\s*\1\s*,' source
source/libads/kerberos.c:   fname = talloc_asprintf(dname, 
"%s/krb5.conf.%s", dname, domain);
$

Perhaps adding a \b to the front of the regexp would be appropriate?

-- 
Steve Langasek   Give me a lever long enough and a Free OS
Debian Developer   to set it on, and I can move the world.
Ubuntu Developerhttp://www.debian.org/
slanga...@ubuntu.com vor...@debian.org


-- 
To UNSUBSCRIBE, email to debian-devel-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org

Re: mass bug filing for undefined sn?printf use

[Michal Čihař]

Hi

Dne Sun, 28 Dec 2008 09:53:40 +0100
Adeodato Simó  napsal(a):

> Michal Čihař 
>gammu

Affected code is only in some example, however I will fix it upstream...

-- 
Michal Čihař | http://cihar.com | http://blog.cihar.com


signature.asc
Description: PGP signature

Re: mass bug filing for undefined sn?printf use

[Steve Langasek]

On Sun, Dec 28, 2008 at 12:42:46AM -0800, Kees Cook wrote:
> And, a possible solution from Anders Kaseorg...
>  This example sprintf() call could be fixed as follows:
>   -sprintf(buf, "%s plus %d", buf, k);
>   +sprintf(buf + strlen(buf), " plus %d", k);
>  Similarly, an invalid snprintf() call could be fixed as follows:
>   -snprintf(buf, buflen, "%s plus %d", buf, k);
>   +snprintf(buf + strlen(buf), buflen - strlen(buf), " plus %d", k);

> Attached is a list of affected packages, generated via:

>   pcregrep -M 'sprintf\s*\(\s*([^,]*)\s*,\s*"%s[^"]*"\s*,\s*\1\s*,'
>   pcregrep -M 'snprintf\s*\(\s*([^,]*)\s*,[^,]*,\s*"%s[^"]*"\s*,\s*\1\s*,'

I would note that this regexp, and the proposed solution, will not match
i18nized format strings; i.e.,

  sprintf(buf, _("%s plus %d"), buf, k);

I don't know whether these are also a problem in practice - but if so, using
sprintf(buf + strlen(buf) [...]) is definitely wrong.

-- 
Steve Langasek   Give me a lever long enough and a Free OS
Debian Developer   to set it on, and I can move the world.
Ubuntu Developerhttp://www.debian.org/
slanga...@ubuntu.com vor...@debian.org


-- 
To UNSUBSCRIBE, email to debian-devel-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org

Re: mass bug filing for undefined sn?printf use

[Cyril Brulebois]

Adeodato Simó  (28/12/2008):
> > Attached is a list of affected packages,
> Cyril Brulebois 
>blender

Bleh… Already pointed out upstream some time ago, but since they don't
care about security at all… I guess I'll have to maintain another sec
patch for years…

>desmume (U)

Evgeni might take care of this one, I think he's very close to upstream.

Mraw,
KiBi.


signature.asc
Description: Digital signature

Re: mass bug filing for undefined sn?printf use

[José Luis Tallón]

Kees Cook wrote:
> Hi,
>
> I'd like to seek advice before I perform a mass-bug filing for this
> unstable (though semi-common) use of "sprintf" and "snprintf":
>
> sprintf(buf, "%s foo %d %d", buf, var1, var2);
>
> This is used in many upstreams to perform a format-string-handling
> version of strcat.
>
> This was originally noticed by Anders Kaseorg in Ubuntu[1], since
> -D_FORTIFY_SOURCE=2 triggers a change in behavior (buf is truncated before
> handling the rest of the format string instead of performing the concat).
>
> Upstream glibc points out[2] that using sprintf in this way is undefined
> under C99, and the man pages have now been updated[3] to reflect this.
> (Though I believe it is possible to patch glibc to avoid the change in
> behavior, it's probably best to work on fixing all the upstreams.)
>
> In Debian, some tools already compile natively with -D_FORTIFY_SOURCE=2,
> and some have Build-Depends on "hardening-wrapper", which enables this
> compiler flag.  As such, it seems sensible to have all affected packages
> fixed since the results of such a call could change.  (Though it is not an
> RC issue.)
>
> And, a possible solution from Anders Kaseorg...
>  This example sprintf() call could be fixed as follows:
>   -sprintf(buf, "%s plus %d", buf, k);
>   +sprintf(buf + strlen(buf), " plus %d", k);
>  Similarly, an invalid snprintf() call could be fixed as follows:
>   -snprintf(buf, buflen, "%s plus %d", buf, k);
>   +snprintf(buf + strlen(buf), buflen - strlen(buf), " plus %d", k);
>
> Attached is a list of affected packages, generated via:
>
>   pcregrep -M 'sprintf\s*\(\s*([^,]*)\s*,\s*"%s[^"]*"\s*,\s*\1\s*,'
>   pcregrep -M 'snprintf\s*\(\s*([^,]*)\s*,[^,]*,\s*"%s[^"]*"\s*,\s*\1\s*,'
>
> The logs for individual packages can be seen here[4].  I've tried to trim
> out stuff that was Ubuntu-specific or not relevant, so apologies in advance
> if there are incorrect (or missing) things in the list.
>   
For LCDproc (as of 5.2), the only matching line is:

contrib/interface-demo2/if_demo.c:sprintf(buffer,
"%s,%s", buffer, if_list[i].if_name);

which resulting code is not even installed in binary form.
> Thoughts?
>
> Thanks,
>   
Thank you for taking the time to investigate this issue.


-- 
To UNSUBSCRIBE, email to debian-devel-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org

Re: mass bug filing for undefined sn?printf use

[Thomas Viehmann]

Kees Cook wrote:
> Attached is a list of affected packages, generated via:
> 
>   pcregrep -M 'sprintf\s*\(\s*([^,]*)\s*,\s*"%s[^"]*"\s*,\s*\1\s*,'
>   pcregrep -M 'snprintf\s*\(\s*([^,]*)\s*,[^,]*,\s*"%s[^"]*"\s*,\s*\1\s*,'
> 
> The logs for individual packages can be seen here[4].  I've tried to trim
> out stuff that was Ubuntu-specific or not relevant, so apologies in advance
> if there are incorrect (or missing) things in the list.
> 
> Thoughts?

How about either matching stuff against the build logs or recompiling
with a compiler that actually fails when asked to compile a file that
matches? That would seem to have potential for reducing the number of
false positives.

Kind regards

T.
-- 
Thomas Viehmann, http://thomas.viehmann.net/


-- 
To UNSUBSCRIBE, email to debian-devel-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org

Re: mass bug filing for undefined sn?printf use

[Tzafrir Cohen]

On Sun, Dec 28, 2008 at 12:42:46AM -0800, Kees Cook wrote:

> asterisk

The relevant code is only:

  channels/misdn_config.c:
sprintf(tempbuf, "%s%s, ", tempbuf, iter->msn);

chan_misdn is not built on the the Debian package (though IIRC it is 
built in some unofficial builds). Nevertheless the code exists in 
upstream trunk and should be fixed.

> iaxclient

A single hit in simpleclient/WinIAX/WinIAX.cpp . That's a sample win32 
code, and I guess we don't build it yet.

-- 
Tzafrir Cohen | tzaf...@jabber.org | VIM is
http://tzafrir.org.il || a Mutt's
tzaf...@cohens.org.il ||  best
ICQ# 16849754 || friend


-- 
To UNSUBSCRIBE, email to debian-devel-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org

Re: mass bug filing for undefined sn?printf use

[Julien BLACHE]

Adeodato Simó  wrote:

Hi,

> Julien BLACHE 
>unpaper

Patch sent.

JB.

-- 
 Julien BLACHE - Debian & GNU/Linux Developer -  
 
 Public key available on  - KeyID: F5D6 5169 
 GPG Fingerprint : 935A 79F1 C8B3 3521 FD62 7CC7 CD61 4FD7 F5D6 5169 


-- 
To UNSUBSCRIBE, email to debian-devel-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org

Re: mass bug filing for undefined sn?printf use

[Neil Williams]

On Sun, 28 Dec 2008 00:42:46 -0800
Kees Cook  wrote:

> I'd like to seek advice before I perform a mass-bug filing for this
> unstable (though semi-common) use of "sprintf" and "snprintf":
> 
> sprintf(buf, "%s foo %d %d", buf, var1, var2);
> 
> This is used in many upstreams to perform a format-string-handling
> version of strcat.
> 
> This was originally noticed by Anders Kaseorg in Ubuntu[1], since
> -D_FORTIFY_SOURCE=2 triggers a change in behavior (buf is truncated before
> handling the rest of the format string instead of performing the concat).
> 
> Upstream glibc points out[2] that using sprintf in this way is undefined
> under C99, and the man pages have now been updated[3] to reflect this.
> (Though I believe it is possible to patch glibc to avoid the change in
> behavior, it's probably best to work on fixing all the upstreams.)
> 
> In Debian, some tools already compile natively with -D_FORTIFY_SOURCE=2,
> and some have Build-Depends on "hardening-wrapper", which enables this
> compiler flag.  As such, it seems sensible to have all affected packages
> fixed since the results of such a call could change.  (Though it is not an
> RC issue.)

By all affected packages, do you mean packages that use the code or
packages that use the code *AND* compile with  or
Build-Depend on hardening-wrapper?

IMHO any bugs filed merely due to the presence of the code without the
means to trigger the error in normal builds should be wishlist.

Re:
Debian GPE team 
   gpe-conf (U)

gpe-conf, being Gtk+ and therefore GLib can simply switch to using
g_strconcat or g_sn?printf or g_strdup_printf and avoid all these
problems. In the specific case of gpe-conf, only two of the files using
this code do not already include gtk/gtk.h or glib/glib.h so it is only
sensible to use the GLib functions instead for most if not all
occurrences. (Indeed, in many cases, the use of a newly allocated
string to be freed later, instead of a static fixed buffer has other
benefits elsewhere.)

> And, a possible solution from Anders Kaseorg...
>  This example sprintf() call could be fixed as follows:
>   -sprintf(buf, "%s plus %d", buf, k);
>   +sprintf(buf + strlen(buf), " plus %d", k);
>  Similarly, an invalid snprintf() call could be fixed as follows:
>   -snprintf(buf, buflen, "%s plus %d", buf, k);
>   +snprintf(buf + strlen(buf), buflen - strlen(buf), " plus %d", k);
> 
> Attached is a list of affected packages, generated via:
> 
>   pcregrep -M 'sprintf\s*\(\s*([^,]*)\s*,\s*"%s[^"]*"\s*,\s*\1\s*,'
>   pcregrep -M 'snprintf\s*\(\s*([^,]*)\s*,[^,]*,\s*"%s[^"]*"\s*,\s*\1\s*,'
> 
> The logs for individual packages can be seen here[4].  I've tried to trim
> out stuff that was Ubuntu-specific or not relevant, so apologies in advance
> if there are incorrect (or missing) things in the list.
> 
> Thoughts?

Split the list according to packages that merely match the regexp and
those that match the regexp *AND* match a second regexp indicating that
the build system either uses -D_FORTIFY_SOURCE=2 or hardening-wrapper?

-- 


Neil Williams
=
http://www.data-freedom.org/
http://www.nosoftwarepatents.com/
http://www.linux.codehelp.co.uk/



pgpC1REHsLAUm.pgp
Description: PGP signature

Re: mass bug filing for undefined sn?printf use

[David Paleino]

On Sun, 28 Dec 2008 09:53:40 +0100, Adeodato Simó wrote:

> > Attached is a list of affected packages,
> 
> Piping through dd-list(1) gives:
> 
> [..]
> 
> Debian-Med Packaging Team 
>ctn
>mafft
> 
> Andreas Tille 
>ctn (U)
> 
> David Paleino 
>mafft (U)
>
> Charles Plessy 
>mafft (U)

As regards ctn, it is from a bundled app we're not building (nor upstream's 
Makefiles do):

$ pcregrep -rnM 'sprintf\s*\(\s*([^,]*)\s*,\s*"%s[^"]*"\s*,\s*\1\s*,' .
./apps/spray_image/spray_image.c:465:return sprintf(uid, "%s.%d", uid, 
studyNum);
./apps/spray_image/spray_image.c:471:return sprintf(uid, "%s.%d.%d", uid, 
studyNum, seriesNum);
./apps/spray_image/spray_image.c:477:return sprintf(uid, "%s.%d.%d.%d", 
uid, studyNum, seriesNum, instanceNum);
$ grep -inR spray_image *
apps/spray_image/Makefile:3:NAME = spray_image
apps/spray_image/Makefile:47:   ./spray_image -q -r -a DRNO -c DRNO drno 2100 
a.dcm
apps/spray_image/spray_image.c:55:** Source File:   $RCSfile: 
spray_image.c,v $
apps/spray_image/spray_image.c:60:static char rcsid[] = "$Revision: 1.4 $ 
$RCSfile: spray_image.c,v $";
$ apt-file list ctn | grep spray
$ 

mafft has been fixed in Debian-Med's svn, thank you.

Kindly,
David

-- 
 . ''`.  Debian maintainer | http://wiki.debian.org/DavidPaleino
 : :'  : Linuxer #334216 --|-- http://www.hanskalabs.net/
 `. `'`  GPG: 1392B174 | http://snipr.com/qa_page
   `-   2BAB C625 4E66 E7B8 450A C3E1 E6AA 9017 1392 B174


signature.asc
Description: PGP signature

Re: mass bug filing for undefined sn?printf use

[Miguel Figueiredo]

Dom, 2008-12-28 às 00:42 -0800, Kees Cook escreveu:
> Hi,
> 
> I'd like to seek advice before I perform a mass-bug filing for this
> unstable (though semi-common) use of "sprintf" and "snprintf":
> 
> sprintf(buf, "%s foo %d %d", buf, var1, var2);
> 
> This is used in many upstreams to perform a format-string-handling
> version of strcat.

[...]

This will be reported upstream?


-- 
To UNSUBSCRIBE, email to debian-devel-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org

Re: mass bug filing for undefined sn?printf use

[Josselin Mouette]

Le dimanche 28 décembre 2008 à 09:53 +0100, Adeodato Simó a écrit :
>gnome-games

They are from the bundled copy of gnuchess, which is not built.

-- 
 .''`.
: :' :  We are debian.org. Lower your prices, surrender your code.
`. `'   We will add your hardware and software distinctiveness to
  `-our own. Resistance is futile.


signature.asc
Description: Ceci est une partie de message	numériquement signée

Re: mass bug filing for undefined sn?printf use

[Mike Hommey]

On Sun, Dec 28, 2008 at 09:53:40AM +0100, Adeodato Simó wrote:
> Mike Hommey 
>xulrunner

./xulrunner-1.8.1.16+nobinonly/toolkit/mozapps/installer/unix/wizard/nsXIEngine.cpp:
sprintf(libpath, "%s/%s", libpath, XPISTUB);
./xulrunner-1.8.1.16+nobinonly/xpinstall/wizard/unix/src2/nsXIEngine.cpp:
sprintf(libpath, "%s/%s", libpath, XPISTUB);

We don't care for these, they are not built.

Mike


-- 
To UNSUBSCRIBE, email to debian-devel-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org

Re: mass bug filing for undefined sn?printf use

[Adeodato Simó]

> Attached is a list of affected packages,

Piping through dd-list(1) gives:

Daniel Leidert (dale) 
   gabedit (U)
   openbabel (U)

Laszlo Boszormenyi (GCS) 
   cdw
   sidplay
   sidplay-libs

Adam Cécile (Le_Vert) 
   aqualung
   audacious-plugins (U)

Masayuki Hatta (mhatta) 
   abiword
   ebview
   insight

Nicolas FRANCOIS (Nekral) 
   shadow (U)

J.H.M. Dassen (Ray) 
   scrollkeeper (U)

Jari Aalto 
   wmfrog

Tim Abbott 
   symmetrica

Moray Allan 
   gpe-conf (U)

Bill Allombert 
   pari

Per Andersson 
   micro-evtd

Domenico Andreoli 
   curl

Hakan Ardo 
   binutils-avr
   gdb-avr

Ben Armstrong 
   xpilot-ng

maximilian attems 
   linux-2.6 (U)

Michael Banck 
   gridengine (U)
   openbabel (U)

Karl Bartel 
   black-box
   penguin-command

Andreas Barth 
   db4.2 (U)

Daniel Baumann 
   tack

Christian Bayle 
   gatos

Christoph Berg 
   oftc-hybrid

Armin Berres 
   kdeedu (U)

Sylvain Beucler 
   freedink (U)

Stephen Birch 
   xball

Julien BLACHE 
   unpaper

Bastian Blank 
   linux-2.6 (U)

Phil Blundell 
   prismstumbler

Phil Blundell 
   gpe-conf (U)

A. Maitland Bottoms 
   vtk

Gonéri Le Bouder 
   barrage (U)
   starfighter (U)

Fathi Boudra 
   kdeedu (U)

Alan Boudreault 
   mapserver (U)

Nicholas Breen 
   gromacs (U)

Ludovic Brenta 
   gnat-gps

Rogério Brito 
   avr-evtd

Cyril Brulebois 
   blender
   desmume (U)

Krzysztof Burghardt 
   xawtv

Daniel Burrows 
   criticalmass

Paul Cager 
   afnix

Ondrej Certik 
   openmx (U)
   paraview (U)

Christian Holm Christensen 
   root-system

Tzafrir Cohen 
   asterisk (U)

Adam Conrad 
   db4.2 (U)
   samba (U)

Arnaud Cornet 
   ircd-ratbox

Leo Costela 
   tcptrack

Julien Cristau 
   libx11 (U)

Marco d'Itri 
   ifmail

Joost Yervante Damad 
   timidity

Matthew Danish 
   sdlperl (U)

Julien Danjou 
   tetrinetx

LI Daobing 
   liblunar
   openbabel (U)

Debian ACE+TAO maintainers 
   ace

Debian Audacious Packagers 
   audacious-plugins

Debian Berkeley DB Maintainers 
   db4.2

Debian Evolution Maintainers 
   evolution-data-server

Debian Games Team 
   barrage
   billard-gl
   desmume
   freedink
   plib (U)
   starfighter
   xbill
   xgalaga

Debian GCC Maintainers 
   gcc-3.3
   gcc-3.4
   gcc-4.1
   gcc-4.2
   gcc-4.3
   gcc-snapshot

Debian GIS Project 
   gdal
   gmt
   grass
   mapserver
   ogdi-dfsg

Debian GNOME Maintainers 
   gnome-games (U)
   scrollkeeper (U)

Debian GPE team 
   gpe-conf (U)

Debian Grid Engine Maintainers 
   gridengine

Debian Kernel Team 
   linux-2.6

Debian multimedia packages maintainers 

   vlc

Debian MySQL Maintainers 
   mysql-dfsg-5.0

Debian Nagios Maintainer Group 
   nagios-plugins

Debian Perl Group 
   libpar-packer-perl

Debian Qt/KDE Maintainers 
   kdeedu

Debian Ruby Extras Maintainers 

   libgsl-ruby (U)

Debian Samba Maintainers 
   samba

Debian Scientific Computing Team 
   openmx
   paraview

Debian SDL packages maintainers 
   sdlperl

Debian VDR Team 
   vdr-plugin-weather
   vdr-plugin-xineliboutput

Debian VoIP Team 
   asterisk
   iaxclient

Debian X Strike Force 
   libx11

Debian Xfce Maintainers 
   xfce4-mpc-plugin

Debian-Med Packaging Team 
   ctn
   mafft

Debichem Team 
   gabedit
   gromacs
   openbabel

Barry deFreese 
   barrage (U)
   billard-gl (U)
   xbill (U)

Murat Demirten 
   ettercap

Mattia Dongili 
   user-mode-linux (U)

Ludovic Drolez 
   swish-e

Sebastian Dröge 
   gnome-games (U)

Bernd Eckenfels 
   ircii

Mark W. Eichin 
   owl

Peter Eisentraut 
   psqlodbc
   slony1

Rene Engelhard 
   kover

Carey Evans 
   tn5250

Bartosz Fenski 
   billard-gl (U)
   libstatgrab
   starfighter (U)

Sean Finney 
   mysql-dfsg-5.0 (U)
   nagios-plugins (U)

Pedro Fragoso 
   evolution-data-server (U)

Bdale Garbee 
   xtrkcad

Hector Garcia 
   mindi-busybox (U)
   mondo (U)

David Moreno Garza 
   gcolor2 (U)

Ionut Georgescu 
   grace

Pascal Giard 
   desmume (U)

Thomas Girard 
   ace (U)

Oystein Gisnas 
   evolution-data-server (U)

Kevin Glynn 
   mozart

Rudy Godoy 
   xfce4-mpc-plugin (U)

John Goerzen 
   libcdk5

Evgeni Golov 
   desmume (U)

Andreas "Jimmy" Gredler 
   gcom

Tobias Grimm 
   vdr-plugin-xineliboutput (U)

Tobias Grimm 
   vdr-plugin-weather (U)

Debian QA Group 
   adplug
   gamix
   gdb-m68hc1x
   gtk-imonc
   htdig
   mp3splt
   pload
   plotmtv
   sqlrelay
   tcpick
   tgif
   ude
   varkon
   vbpp
   xmcd

Yu Guanghui 
   unicon

Aurélien GÉRÔME 
   restartd

Aurélien GÉRÔME 
   ircd-hybrid (U)

Thomas Günther 
   vdr-plugin-weather (U)
   vdr-plugin-xineliboutput (U)

Henrique Haas 
   trueprint

Steve Halasz 
   gdal (U)
   grass (U)

Christian Hammers 
   mysql-dfsg-5.0 (U)

Sam Hartman 
   barnowl

Heikki Henriksen 
   evolution-data-server (U)

M. Alex Hermosilla 
   multi-aterm

gregor herrmann 
   libpar-packer-perl (U)

Mike Hommey 
   xulrunner

Simon Huggins 
   xfce4-mpc-plugin (U)

Zephaniah E. Hull 
   sdlperl (U)

Mark Hymers 
   gridengine (U)

Ervin Hearn III 
   pennmush

Damyan Ivanov 
   libpar-pa