[donatj]

You can get as many eyes as you want on all changes. I don't think it will help much. This sort of thing usually happens because of a change in something completely benign, rather than "security-critical changes".

We recently had a pretty major vulnerability exposed by a PEN test (thankfully) that was caused by a single misplaced NOT ! operator in a pretty simple function, maybe 20loc with 100% test coverage as counted by line.

The problem case was untested because while tests touched 100% of lines, not all combinations of paths through the code were tested.

The code had been written by myself about a year ago, and reviewed by four seasoned developers, none of whom spotted the issue.

How many thousands of eyes are on OpenSSL and yet major world ending vulnerabilities still crop up?

This might be a hot take, but you can make the process as painful as you like but at the end of the day, building secure software is nigh impossible. We should be putting less trust in software.

[OptionOfT]

Would branch coverage have helped you here?

And I think one of the approaches that helped me to write safer code is to parse, and not validate.

That way drastically limit the situations we can get into where we forget to validate certain conditions.

e.g. you have a User struct, and you want to do an action which requires you to validate whether the user is an admin.

2 options here

* validate whether the user is an admin (which could happen multiple times when you're invoking distinct functions as part of a workflow)

* parse the User into an AdminUser. If the user is an admin, the function will work, and then you can pass on your new struct into places that require an admin. If it fails, the user is not an admin. Now you have merged all your checks into 1 place.

[donatj]

Yeah, almost certainly. We had looked into enabling branch detection several years ago, but it had slowed our suite down to the point where we were not sure it was worth it. Maybe worth looking into again.

I do generally like that technique of returning more vetted objects, but I'm not sure that it scales. Our users have close to one hundred different permissions based on the features their admin pays for, having an object for every type seems like a lot. Every combination of types is straight out.

The original issue I was talking about I don't think could have been helped. It was an encoding problem that could have potentially lead to XSS injection with a very specific set of GET parameters. I was frankly surprised that the PEN testers managed to find it.

[wepple]

Not a hot take at all, for anyone who has worked with securing code.

SWEs simply aren’t trained to deeply examine code and the side effects of it being pressured by skilled attackers.

2+ LGTMs reduces the change of a security issue making its way in, but no amount of expensive “more eyes” will eradicate bugs.

[dopylitty]

> We should be putting less trust in software.

This is the conclusion I've come to as well. Nothing critical should be solely software dependent. Software will fail and critical systems should be able to function without it.