Hello, and welcome to C++ Weekly. I am your host, Jason Turner. I'm available for code reviews and
on-site training. There was recently a jailbreak released for the PlayStation 5. Now I am here to
tell you that this really never should have happened and was trivially easy to prevent.
So you may be aware that one of the main things that I do is teach best practices
to C++ programmers, and while this was a C vulnerability, this easily applies to
C/C++ equally well; and we can see here that the vulnerability here listed collected a bounty of
$10,000 US dollars. That's pretty good. Let's take a quick look at what this
vulnerability actually entailed. So the main problem is a incorrect
calculation of buffer size that came down to a type mismatch. So if we look over here
we can see that we have calculated a size as a size_t, and that size is passed to this function
called sceFatfsCreateHeapV1. The fundamental fall comes here when we actually look at the
type that this parameter expects - it expects an 'int', so fundamentally we have a type mismatch.
These things are not the same. It's possible to pass in some sort of value that overflows
and then this malloc does the wrong thing. This code never should have been written this way,
and I'll demonstrate how it would have been trivially easy for the engineers working on
this to have avoided this particular problem. So I have up here my sandbox that I like to
use inside of Compiler Explorer, but before I continue I would also like to point out
that you should really check out the video description for a link to my book on C++
Best Practices where you can learn an awful lot more about this kind of topic and how
to prevent errors like this. So they had some function that calculated a size_t...
We're just gonna stub out this example. So I have a function that gets a size,
I store it in a local variable; it's of type size_t. (Now this value is not currently being
used and there is no definition for getSize, so the program can't actually execute, so I'm
just going to go ahead and disable execution so we can look at the analysis of the code
itself here.) This value is passed to another function that, let's just call it allocate,
and it took some sort of size. Let's go ahead and flip back real quick to the actual bug report...
Now I don't know what version of malloc they are calling here
because malloc from the C standard library takes exactly one parameter; so we're going to use that
version. So they had an alloc function of some sort and it did extra work for them,
but they ended up with something approximately like this: they have some sort of calculation that
calculates a size_t, they then pass that size_t to this allocate function that did the actual work,
and that calls malloc and returns some result. Pretty straightforward, simple example here.
Now you will note that at the moment I don't have any particular warning flags enabled in
my compiler, but I'm going to go ahead and give them the benefit of the doubt and turn on -Wall,
and then I'm going to turn on -Wextra, which I find many developers don't even go the step of
-Wextra, but I will tell you there is one warning that I have a very difficult time
convincing developers to use. I get told things like, "Oh, this warning creates too much noise,
false positives, meaningless;" or, "I know better than the compiler," something like "C and C++ are
useless if I enable this" because they've gotten so used to writing code like this;
but I will assure you that this error does not, in fact, have any of these problems. It does not
create too much noise. It does not create false positives. It is not meaningless, and in fact,
enabling this warning on your builds can catch actual bugs that are actual security flaws,
and it is called -Wconversion. This warns you whenever you are calling a function,
or in any other way, invoking a lossy conversion, and this conversion that we have
is this conversion from size_t to a function expecting an 'int'. So let's go ahead and fix
this, and as you can see it fully solved the problem because we have a size_t passed to a
function that expects a size_t, passed to a function that expects a size_t, and it's just that
simple. So you should go enable -Wconversion on your warning set if you don't currently have it.
I personally consider the set of warnings -Werror, -Wall, -Wextra and -Wconversion to be the absolute
minimum set of warnings that you should have in your code; and you should hire me to come
to your company to talk more about best practices and how you can write code that avoids this kind
of security flaw. So thank you for watching this episode of C++ Weekly, and be sure to subscribe.