std.file functions and embedded NUL characters [CWE-158] (page 4)

Am 31.07.25 um 23:47 schrieb Richard (Rikki) Andrew Cattermole: > This is quite a good example of why for PhobosV3 I want us to go through a FilePath abstraction, rather than accepting random strings for file names. > > This is indeed a security vulnerability, but it isn't on D's side. > All system API's take in a null terminated string, when it should've been pointer + length. > > If someone has a problem with this currently, you can call ``isValidPath`` in ``std.path``, which will check for the null character. > > https://dlang.org/phobos/std_path.html#isValidPath I strongly suggest to also at least distinguish between Windows and Posix path formats to avoid ambiguity issues. vibe.core.path defines `WindowsPath`, `PosixPath` and `InetPath`, as well as an OS dependent alias `NativePath` (aliases to either `WindowsPath` or `PosixPath`) to avoid having to think about this all the time. However, conversions between path formats are always explicit, with the appropriate validation taking place.

4 days ago

Re: std.file functions and embedded NUL characters [CWE-158]

Posted by kdevel
in reply to Steven Schveighoffer

Permalink

kdevel

Posted in reply to Steven Schveighoffer

Permalink

On Sunday, 3 August 2025 at 21:01:05 UTC, Steven Schveighoffer wrote:

On Sunday, 3 August 2025 at 00:03:47 UTC, kdevel wrote:

On Friday, 1 August 2025 at 17:53:17 UTC, Steven Schveighoffer wrote:

[...]
It's always a fight between correctness and performance here.

Pardon?

I mean in terms of where you apply the checks. You can say "I expect you to have validated this before sending it in", or you can say "I will always validate this, even if you already have, in the name of correctness."

And I suppose a better word should have been "tradeoff" and not "fight".

I like the fragment "[...] it has to be correct before being fast."

[...]
The issue is going to be fixed, not sure if you saw my issue report. It's quite an easy fix actually.

Thanks but sorry I could not yet take a look.

But just so you know, C also allows passing in C strings with embedded null characters:

#include <unistd.h>

int main() {
   rmdir("hello\0world");
   return 0;
}

On the one hand there is no such thing as a C string with embedded NUL (because the NUL terminates the string).

On the other hand the rmdir function takes const char *pathname. Your code contains an array argument ("hello\0world"). According to C's rules this array decays into a pointer before rmdir is invoked. Thus one can hardly say that a string with embedded NUL is passed to the function.

And we do expose core.stdc.posix.unistd.

Yes. But in the D-function in question is std.file.rmdir. I copied the relevant code:

import std.stdio : writeln;
import std.range : isSomeFiniteCharInputRange;
import std.traits : isConvertibleToString;

void rmdir (R) (R pathname)
if (isSomeFiniteCharInputRange!R && !isConvertibleToString!R)
{
   writeln (__PRETTY_FUNCTION__);
   writeln (typeid (pathname));
   writeln (pathname.length);
   writeln (pathname);
}

void rmdir(R)(auto ref R pathname)
if (isConvertibleToString!R)
{
   writeln (__PRETTY_FUNCTION__);
   writeln (typeid (pathname));
   writeln (pathname.length);
   writeln (pathname);
}

void main ()
{
   rmdir ("a\0b");
}

which prints

void rmdir.rmdir!string.rmdir(string pathname)
immutable(char)[]
3
ab   <--- there is a NUL between a and b

I.e. the function really takes the string with the embedded NUL.

On Tuesday, 5 August 2025 at 02:47:22 UTC, kdevel wrote:

On Sunday, 3 August 2025 at 21:01:05 UTC, Steven Schveighoffer wrote:

[...]
The issue is going to be fixed, not sure if you saw my issue report. It's quite an easy fix actually.

Thanks but sorry I could not yet take a look.

TL;DR: We can fix tempCString since we are always copying the whole thing. So the issue is moot at that point.

> >

But just so you know, C also allows passing in C strings with embedded null characters:

#include <unistd.h>

int main() {
   rmdir("hello\0world");
   return 0;
}

On the one hand there is no such thing as a C string with embedded NUL (because the NUL terminates the string).

C does not give me an error for this code. Regardless of what it "technically" is doing, you have the same issue there -- C does not warn you or error on this concern. So it is identically vulnerable to your test case.

> >

And we do expose core.stdc.posix.unistd.

Yes. But in the D-function in question is std.file.rmdir.

C already needs to caution people not to call this function with embedded null string array. D can also do the same thing. My point is, I don't imagine we need to "fix" the unistd binding.

Again, who's responsible for verification is a tradeoff. In this case, we have an obvious answer for std.file.rmdir, because we can fix tempCString. But it also could be solved by just identifying what happens if you pass in such a string, and put the onus on the user to check.

-Steve

Forums