Typical security issues in C++: why the GC isn't your enemy

[Siarhei Siamashka]

On Monday, 12 December 2022 at 03:48:26 UTC, Walter Bright wrote:
> On 12/9/2022 2:39 PM, H. S. Teoh wrote:
>> According to `dmd -h`, they are disabled everywhere except 
>> @safe code.
>
> Safe code is where bounds checking is needed.
>
>> And arguably, @system code is precisely where you WANT to have 
>> bounds
>> checks.
>
> Use of @system code should be minimized and in it you're 
> allowed to do whatever you want.

You have my 100% support here. But the tricky thing is that the 
majority of the current D language community seems to disagree 
with us, as evidenced by the apparent rejection of DIP 1028. And 
we should do a much better job convincing them to change their 
mind.

What kind of dangers are threatening the users of @system code? 
This needs to be explained better. Especially considering that 
every beginner pretty much starts using @system code by default 
even before knowing that there's a choice.

=== use after free ===

Right from the top of the Chromium list, the "use after free" 
security problem. Have a look at [this forum 
thread](https://forum.dlang.org/post/wssohnwydjgsouywbitk@forum.dlang.org). Presumably a beginner is asking about how to deal with a c-string returned by the sqlite3 library. And many of the forum regulars are very happily suggesting to use `std.conv.fromStringz` or some other ways of turning the pointer into a slice while avoiding a GC allocation. One person even believed that `std.conv.fromStringz` is doing a GC allocation. Now if this string is saved somewhere instead of just printing it, then we potentially have a "use after free" problem, because this memory is managed by sqlite3 and will get a `realloc` treatment by the follow-up sqlite3 API calls (I may be wrong, but that's what it looks like after quickly skimming through the sqlite3 source code). This is a potentially dangerous stuff and it mostly affects the @system code because the compiler remains silent about a lot of fishy things.

Also see the https://github.com/dlang/dmd/pull/14639 pull 
request. Some of the functions used to be incorrectly annotated 
as @trusted and this was fixed later.

The users (and especially beginners!) need to annotate their code 
as @safe before receiving any help from the compiler related to 
this problem.

=== references to stack variables escaping the scope ===

Here's a good example 
https://forum.dlang.org/post/duwrxnkjaafnzpfgnted@forum.dlang.org 
posted by someone, who is not a complete beginner (I can see 
their post 
https://forum.dlang.org/post/pdjvwvzysetndrtkxmea@forum.dlang.org 
from many months earlier). Looks like this person was not aware 
of the @safe attribute all this time. And possibly started 
experimenting and asking questions only after running into 
troubles and having to debug some real code.

Another interesting observation is that despite my recommendation 
to add "@safe:" at the top of the source file, this person still 
plastered the @safe attribute all over the place in the next code 
snippet. Also not recognizing this syntax doesn't seem uncommon: 
https://forum.dlang.org/post/ddhxlvprhdpqrhkbxuyb@forum.dlang.org

=== bounds checking ===

Bounds checking is not done in @safe and @trusted code in 
"-release" builds. How long does it typically take for a beginner 
to learn about this? And how many of them learn it proactively by 
reading the tutorials/documentation rather than getting 
surprised/ambushed by an unexpected segfault in their release 
build? This reminds me about 
https://forum.dlang.org/post/couzhdooeskwppuklasf@forum.dlang.org

Now let's look at the DUB ecosystem and what kind of options are 
used for compiling the existing DUB packages. Running `dub build 
--help` shows:

     -b  --build=VALUE     Specifies the type of build to perform. 
Note that
                           setting the DFLAGS environment variable 
will override
                           the build type with custom flags.
                           Possible names:
                             debug, plain, release, release-debug,
                             release-nobounds, unittest, profile, 
profile-gc,
                             docs, ddox, cov, unittest-cov, syntax 
and custom
                             types

Trying these build types in verbose mode reveals that the 
following command line options are supplied to DMD:

     debug            = "-debug -g -w ..."
     plain            = "-w ..."
     release          = "-release -inline -O -w ..."
     release-debug    = "-release -g -inline -O -w ..."
     release-nobounds = "-release -inline -noboundscheck -O -w ..."

Basically `-O` is always bundled together with `-release` and DUB 
packages built with optimizations won't have bounds checking in 
@system code. Of course, unless the packages explicitly override 
this in their sdl/json.


So we are in a rather weird situation. At least in the DUB 
ecosystem a lot of applications and libraries de-facto may have 
unnecessary security issues caused by having too much @system 
code. The community doesn't want to change the default from 
@system to @safe. And at the same time the same community wants 
to promote D as a safe language. People do get upset when, let's 
say, NSA is not listing D as an example of a safe language.

=== how to fix all of this ===

My suggestion is still the same: the compiler should start making 
noise whenever a function gets the @system attribute assigned to 
it by default. Similar to deprecation warnings. This message does 
not have to abort compilation and the existing DUB packages 
should still build successfully (despite the '-w' option). The 
text of the message may look like this:

     "The function 'foobar' got @system attribute by default. If 
this is really what you want, then please add '@system:' at the 
top of the source file or read this <URL> for detailed 
explanations."

I think that this will provide a gentle push/reminder for the 
maintainers of the existing packages. Also beginners will learn 
about the @safe attribute much faster.