Bug#563637: improvements from Ubuntu to handle compiler hardening better
On Thu, Feb 11, 2010 at 11:37:10AM -0800, Kees Cook wrote:
> Hi Aurelien,
>
> On Sun, Feb 07, 2010 at 02:13:08PM +0100, Aurelien Jarno wrote:
> > > I would like to include the following patches that Ubuntu has carried for
> > > several releases now. (Note that submitted-leading-zero-stack-guard.diff
> > > will need to be adjusted slightly if stack-guard-quick-randomization.diff
> > > is not applied.)
> >
> > I have applied the two stack protection patches in the Debian package,
> > but not the two other ones. See my comments below.
>
> Excellent, thanks!
>
> > > no-sprintf-pre-truncate.diff
> > > The sprintf function used when -D_FORTIFY_SOURCE=2 is used incorrectly
> > > pre-truncates the destination buffer; this changes the long-standing
> > > expectation of sprintf(foo,"%sbaz",foo) to work. See the patch for
> > > further discussion.
> >
> > As explained in the bug report, this code is not valid anyway. If we
> > want people to fix their code, we should not workaround the issue. Also
> > I am not able to evaluate the impact on the fix, and don't know if it
> > may introduce a security bug.
>
> Right, it's incorrect, but around 200 packages[1] use it and expect the
> prior behavior. I don't feel there is a security issue here, but I can
> respect not wanting to change it. 200 is a pretty small number of
> packages compared to the overall size of the archive.
>
> Perhaps I can re-scan the archive and actually do the mass bug filing.
We are now more than 5 years since the mail with the list of affected
packages, and -D_FORTIFY_SOURCE=2 is now the default, so I guess all
affected packages have been fixed.
> > > local-fwrite-no-attr-unused.diff
> > > Again, patch contains discussion, but basically, this disables a
> > > useless and noisy warning that -D_FORTIFY_SOURCE=2 triggers.
> >
> > I think people should either not use -D_FORTIFY_SOURCE=2 or fix their
> > code. This is a warning anyway. I agree an error can happens up to the
> > fclose() call, but it's not an excuse to not check possible errors at
> > the fwrite() level. The real bug is actually that fclose() is not marked
> > __wur, and that's probably what has to be fixed.
>
> Yeah, I would tend to agree. The main glitch was that there is no
> compiler option to turn off the warning. :(
This has been done by upstream in glibc 2.16.
So I guess we could now close this bug. Do you agree?
--
Aurelien Jarno GPG: 4096R/1DDD8C9B
aurelien@aurel32.net http://www.aurel32.net
Reply to: