Received-SPF: pass (google.com: domain of linux-kernel+bounces-96413-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Precedence: bulk
MIME-Version: 1.0
References: <20240307223849.13d5b58b@barney> <20240307232927.171197-1-rand.sec96@gmail.com>
In-Reply-To: <20240307232927.171197-1-rand.sec96@gmail.com>
From: James Dutton <james.dutton@gmail.com>
Date: Fri, 8 Mar 2024 01:04:41 +0000
Message-ID: <CAAMvbhE53upfqfB=afJK9YAZUWFTBN8ripae0W+PUUXdwj4-kw@mail.gmail.com>
Subject: Re: [PATCH v3] ssb: Fix potential NULL pointer dereference in ssb_device_uevent
To: Rand Deeb <rand.sec96@gmail.com>
Cc: m@bues.ch, deeb.rand@confident.ru, jonas.gorski@gmail.com, 
	khoroshilov@ispras.ru, kvalo@kernel.org, linux-kernel@vger.kernel.org, 
	linux-wireless@vger.kernel.org, lvc-project@linuxtesting.org, 
	voskresenski.stanislav@confident.ru
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Thu, 7 Mar 2024 at 23:29, Rand Deeb <rand.sec96@gmail.com> wrote:
>
>
> On Fri, Mar 8, 2024 at 12:39=E2=80=AFAM Michael B=C3=BCsch <m@bues.ch> wr=
ote:
>
> > The point is that leaving them in is defensive programming against futu=
re changes
> > or against possible misunderstandings of the situation.
>
> Dear Michael, I understand your point. It's essential to consider defensi=
ve
> 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 w=
hy
> security sucks in today's software."
> I think it's a matter of design problem, when you don't have a good desig=
n
> 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 !=3D 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 !=3D null && x is a number
>                 return x+y;
> }
> MULTIPLY(x, y){
>         if x !=3D 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.