Re: [PATCH v3] ssb: Fix potential NULL pointer dereference in ssb_device_uevent

From: James Dutton
Date: Thu Mar 07 2024 - 20:05:33 EST


On Thu, 7 Mar 2024 at 23:29, Rand Deeb <rand.sec96@xxxxxxxxx> wrote:
>
>
> On Fri, Mar 8, 2024 at 12:39 AM Michael Büsch <m@xxxxxxx> wrote:
>
> > The point is that leaving them in is defensive programming against future changes
> > or against possible misunderstandings of the situation.
>
> Dear Michael, I understand your point. It's essential to consider defensive
> programming principles to anticipate and mitigate potential issues in the
> future. However, it's also crucial to strike a balance and not overburden
> every function with excessive checks. It's about adopting a mindset of
> anticipating potential problems while also maintaining code clarity and
> efficiency.
>
> > > I understand and respect your point of view as software engineer but it's a
> > > matter of design problems which is not our case here.
> >
> > No, it very well is.
>
> I'm talking about your phrase "Not having these checks is a big part of why
> security sucks in today's software."
> I think it's a matter of design problem, when you don't have a good design
> of course you'll need to add so many checks everywhere.
> Let me explain my point of view by example,
>
> // Good design

Note: I am not so sure that this is Good design.

> CHECK(x){
> if x != null && x is a number
> return true;
> else return false;
> }
> MULTIPLY(a, b){
> return a*b;
> }
> SUM(a, b){
> return a+b;
> }
> ....
> MAIN(){
> // input a, b
> CHECK(a);
> CHECK(b);
> // now do the operations
> SUM(a, b)
> MULTIPLY(a, b)
> }
>
> // Bad design
> SUM(x, y){
> if x != null && x is a number
> return x+y;
> }
> MULTIPLY(x, y){
> if x != null && x is a number
> return x*y;
> }
> ...
>

The reason it is probably not a good design is what comes later.
Another developer comes along and says I see a nice SUM(a, b);
function that I would like to re-use in my new function I am adding.
But that new developer introduces a bug whereby they have implemented
their CHECK(a) wrongly which results in SUM(a, b) now being a security
exploit point because of some very subtle bug in CHECK(a) that no one
noticed initially.
After a while, we might have ten functions that all re-use SUM(a, b)
at which point it becomes too time consuming for someone to check all
ten functions don't have bugs in their CHECK(a) steps.
It is always easier for the safety checks to be done as close as
possible to the potential exploit point (e.g. NULL pointer
dereference) so that it catches all future cases of re-use of the
function.
For example, there exist today zero day exploits in the Linux wireless
code that is due to the absence of these checks being done at the
exploit point.
The biggest problem with all this, is if I sutily (without wishing to
give away that it is to fix a zero day exploit) submitted a patch to
do an extra check in SUM(a, b) that I know prevents a zero day
exploit, my patch would be rejected.