Re: git: be04fec42638 - main - Import _FORTIFY_SOURCE implementation from NetBSD

I did not know a battle was even being fought. :-)

I'd like to give a little hypothetical as to why I think these
features are complementary to each other.

The primary goal of SafeStack is to protect the control flow. This
means putting spillable data in a separate stack, named the unsafe
stack. Spilling one buffer into another is (usually) not as bad as
spilling onto the return address pointer on the stack. SafeStack does
not attempt to prevent spillage.

The primary goal of the SSP canary is also to protect control flow.
However, in some cases, the check may be too late. If a function
pointer sits adjacent to a spilled buffer, and the function pointer is
called after the buffer is overflowed, the attacker gains arbitrary
code execution.

One feature critical to _FORTIFY_SOURCE is better bounds checking,
both at compile time and run time.

Let's say, hypothetically, that two buffers are created and sit
adjacent to each other. SafeStack put these two buffers on the unsafe
stack. The second buffer contains authentication and authorization
data. By overflowing into the second buffer, the attacker can gain
new privileges. If the buffer contains authentication data, perhaps
the attacker can (from this point forward) steal the session from
another user. If the buffer contains authorization data, perhaps the
attacker can (from this point forward) perform privileged operations.

This is where _FORTIFY_SOURCE comes in. If _FORTIFY_SOURCE can prevent
the overflow from happening, the attacker cannot steal sessions or
perform privileged operations.

I intentionally skipped over SSP in this hypothetical. In this
particular case, trying to protect the return address pointer saved on
the stack is moot. SSP is still useful, but not for this particular
hypothetical.

I hope this makes sense.

Thanks,

-- 
Shawn Webb
Cofounder / Security Engineer
HardenedBSD

Tor-ified Signal: +1 303-901-1600 / shawn_webb_opsec.50
https://git.hardenedbsd.org/hardenedbsd/pubkeys/-/raw/master/Shawn_Webb/03A4CBEBB82EA5A67D9F3853FF2E67A277F8E1FA.pub.asc

On Sun, May 19, 2024 at 02:47:22PM +0000, Pedro Giffuni wrote:
>  Hmm... well.
> In all honesty I understand I am doomed to lose this battle :).FORTIFY_SOURCE is in linux and in Apple and that weights enough that it had to find it's way to FreeBSD sooner or later. Plus I am just not much involved in FreeBSD or OSs anymore so I don't feel like stopping other people from doing development.
> best of lucks!
> Pedro.
> ps. Just for the reference, the Google guys did develop a document on how they implemented this for clang on bionic:https://goo.gl/8HS2dW
> 
>     On Sunday, May 19, 2024 at 12:13:00 AM GMT-5, Kyle Evans <kevans@freebsd.org> wrote:  
>  
>  On 5/18/24 23:39, Pedro Giffuni wrote:
> > FWIW .. and let me be clear I haven't worked on this in ages and I am 
> > not planning to retake this either...
> > 
> > clang just couldn't do the static  fortify_source checks  due to the way 
> > llvm uses an intermediate representation; the size just couldn't be 
> > handled in the preprocessor. Google did spend some time adding extra 
> > attributes to clang to improve the debugging and you can see that 
> > implemented in bionic libc but that was it. musl didn't even try.
> > 
> 
> Admittedly, I have no idea what you're talking about here; none of this 
> implementation requires any knowledge of anything at preproc time. 
> __builtin_object_size() does the right thing, and the typically 
> performance critical string/memory ops use __builtin___foo_chk() that do 
> successfully get optimized away in the common case to the underlying 
> foo() call.  This all works very well with clang, I haven't tested it 
> under GCC but, as you've noted, would assume that it works at least as well.
> 
> > fortify_source does replace some key libc functions with memory checking 
> > alternatives and that turns out to be annoying when debugging. In a way 
> > it breaks that principle C programmers once had, where developers are 
> > expected to know what they are doing, and if the error is caught at 
> > runtime by the stack protector anyways it ends up being redundant.
> > > One more thing about the static checks. Most of the linux distributions
> > out there indeed have built their software packages with GCC and 
> > fortify_source >=2. As a consequence, when we ran an exp-run on the 
> > ports tree (with GCC), fortify_source didn't find anything: it was 
> > basically a waste of time.
> > 
> > Another reason for not setting it by default is performance. And here I 
> > answer Shawn's comment on why not enable stack-protector-all and 
> > safestack and fortify_source at the same time: running unnecessary 
> > checks over and over again wastes energy and can have some performance 
> > hit. The later may seem negligible in modern processors, but why do them 
> > if they bring no benefit? (No need to answer ... just left as food for 
> > thought)
> > 
> > Pedro.
> > 
> > On Saturday, May 18, 2024 at 09:08:52 PM GMT-5, Kyle Evans 
> > <kevans@freebsd.org> wrote:
> > 
> > 
> > 
> > 
> > On 5/18/24 20:09, Pedro Giffuni wrote:
> >  > (sorry for top posting .. my mailer just sucks)
> >  > Hi;
> >  >
> >  > I used to like the limited static checking FORTIFY_SOURCE provides and
> >  > when I ran it over FreeBSD it did find a couple of minor issues. It only
> >  > works for GCC though.
> >  >
> > 
> > I don't think this is particularly true anymore; I haven't found a case
> > yet where __builtin_object_size(3) doesn't give me the correct size
> > while GCC did.  I'd welcome counter-examples here, though -- we have
> > funding to both finish the project (widen the _FORTIFY_SOURCE net to
> > more of libc/libsys) and add tests to demonstrate that it's both
> > functional and correct.  It would be useful to also document
> > deficiencies in the tests.
> > 
> >  > I guess it doesn't really hurt to have FORTIFY_SOURCE around and NetBSD
> >  > had the least intrusive implementation the last time I checked but I
> >  > would certainly request it should never be activated by default,
> >  > specially with clang. The GCC version has seen more development on glibc
> >  > but I still think its a dead end.
> >  >
> > 
> > I don't see a compelling reason to avoid enabling it by default; see
> > above, the functionality that we need in clang appears to be just fine
> > (and, iirc, was also fine when I checked at the beginning of working on
> > this in 2021) and it provides useful
> > 
> >  > What I would like to see working on FreeBSD is Safestack as a
> >  > replacement for the stack protector, which we were so very slow to adopt
> >  > even when it was originally developed in FreeBSD. I think other projects
> >  > based on FreeBSD (Chimera and hardenedBSD) have been using it but I
> >  > don't know the details.
> >  >
> > 
> > No comment there, though I think Shawn Webb / HardenedBSD had been
> > playing around with SafeStack (and might have enabled it? I haven't
> > actually looked in a while now).
> > 
> >  > This is just all my $0.02
> >  >
> >  > Pedro.
> > 
> > Thanks,
> > 
> > Kyle Evans
> > 
> >  >
> >  > On Saturday, May 18, 2024 at 05:54:42 PM GMT-5, Kyle Evans
> >  > <kaevans@fastmail.com <mailto:kaevans@fastmail.com>> wrote:
> >  >
> >  >
> >  >
> >  >
> >  > On May 18, 2024 13:42, Pedro Giffuni <pfg@freebsd.org 
> > <mailto:pfg@freebsd.org>> wrote:
> >  >
> >  >    Oh no .. please not...
> >  >
> >  >    We went into that in a GSoC:
> >  >
> >  > 
> > https://wiki.freebsd.org/SummerOfCode2015/FreeBSDLibcSecurityExtensions 
> > <https://wiki.freebsd.org/SummerOfCode2015/FreeBSDLibcSecurityExtensions> <https://wiki.freebsd.org/SummerOfCode2015/FreeBSDLibcSecurityExtensions <https://wiki.freebsd.org/SummerOfCode2015/FreeBSDLibcSecurityExtensions>>
> >  >
> >  >
> >  >    Ultimately it proved to be useless since stack-protector-strong.
> >  >
> >  >
> >  > Respectfully, I disagree with your conclusion here:
> >  >
> >  > 1.) _FORTIFY_SOURCE provides more granular detection of overflow; I
> >  > don't have to overflow all the way into the canary at the end of the
> >  > frame to be detected, so my minor bug now can be caught before something
> >  > causes the stack frame to be rearranged and turn it into a security
> >  > issue later
> >  >
> >  > 2.) __builtin_object_size doesn't work on heap objects, but it actually
> >  > can work on subobjects from a heap allocation (e.g., &foo->name), so the
> >  > coverage extends beyond the stack into starting to detect other kinds of
> >  > overflow
> >  >
> >  > While the security value over stack-protector-strong may be marginal (I
> >  > won't debate this specifically), the feature still has value in general.
> >  >
> >  > Thanks,
> >  >
> >  > Kyle Evans
> >  
>