Page MenuHomeFreeBSD

ssp: fix our gets_s implementation under _FORTIFY_SOURCE
ClosedPublic

Authored by kevans on Thu, Apr 30, 4:44 AM.

Details

Summary

Annex K specifies an interface for handling constraint violations from
gets_s, but we previously broke this for some classes of get_s misuse.

Provide a more nuanced version that tries to dodge errors that would
trigger a constraint handler while still providing value. Notably, we
don't want to trigger a failure unless the passed-in length reasonably
fits within an RSIZE_MAX, because gets_s will immediately call larger
lengths bogus and fail.

PR: 294881

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

include/ssp/stdio.h
69

Actually, I think I may backtrack slightly on this: still immediate __chk_fail if len is within bounds, but set a local need_error if we still know len is too high and assert or __chk_fail if it doesn't error out as expected.

Walk it back slightly: if the requested len exceeds RSIZE_MAX, give gets_s
some leeway to handle it properly but crash anyways if gets_s doesn't do the
right thing.

This does mean that we could have overflowed the buffer if it's not handled
properly and all bets are probably off on whether someone could have clobbered
our own stack frame in a way to avoid the impending crash, but hopefully we
would have detected it in a safe environment long before someone finds a way to
craft an exploit around it. This is a scenario we're unlikely to get wrong
anyways.

markj added inline comments.
include/ssp/stdio.h
78

This feels a bit dubious since by this point some hypothetical memory corruption would have already happened, so one can't count on need_fail having been preserved, say. But I'm not sure what else you can do, and if the gets_s() implementation behaves as specified then this isn't a concern.

This revision is now accepted and ready to land.Thu, Apr 30, 5:52 PM
include/ssp/stdio.h
78

Right, that's what I noted in the update comment here, my hope was:

but hopefully we would have detected it in a safe environment long before someone finds a way to craft an exploit around it. This is a scenario we're unlikely to get wrong anyways.

We do have test coverage to be sure that we're handling it in a reasonable manner, so it's really just a layer of paranoia more than anything