News for hackers. Stuff that matters.
Re:Fix the unit test and open a giant hole everywhere by yjftsjthsd-h on Saturday December 25, 2021 @12:47AM (#29680812)
So the funny thing is, when I saw that problem description, my first thought was: "oh, that's what mkdir -p does; I wonder if it would be easier to just shell out to that than try and implement it yourself". I grant that that implementation is a massive SQL-injection-like vulnerability, but I'm still not 100% convinced that the basic impulse is wrong; I can easily imagine an alternate universe where the second half of this blog post is talking about how someone wrote it themselves and introduced some subtle bug that the coreutils folks fixed 20 years ago and everyone else has to painfully rediscover. I wonder if any of the major implementations are friendly to being used as a library? Obviously then you have to deal with licensing issues, which is a catch. Maybe fork+exec? I dunno, I'm just trying to feel out whether there's a middle ground between system() and rewriting functionality that someone else has written and that's been used by a billion other users for decades.
Re:Fix the unit test and open a giant hole everywhere by lawl on Saturday December 25, 2021 @03:11AM (#29681412)
Imo the middle ground here is that this is a common enough operation that it should be in your language's standard library, see e.g. [0][1]. And if you have to use C, then yes, write it yourself or copy some other implementation with a compatible license. That's what you signed up for when you decided you need C.

[0] https://pkg.go.dev/os#MkdirAll

[1] https://doc.rust-lang.org/std/fs/fn.create_dir_all.html

Re:Fix the unit test and open a giant hole everywhere by fivea on Saturday December 25, 2021 @03:54AM (#29681619)
> Imo the middle ground here is that this is a common enough operation that it should be in your language's standard library, see e.g. [0][1].

Is it, though?

I mean, C is around for around half a century, and C is perhaps the primary language in the FLOSS universe.

Still, why is there no widely adopted idempotent directory tree creation library?

Re:Fix the unit test and open a giant hole everywhere by mgaunard on Saturday December 25, 2021 @05:08AM (#29681951)
Because C stopped evolving long ago, in spite of the recent C standards that have more to do with a reanimated zombie than anything else.

In C++ that functionality is indeed part of the standard library.

Re:Fix the unit test and open a giant hole everywhere by vlovich123 on Saturday December 25, 2021 @11:24AM (#29684476)
Let’s not act so high and mighty. It got added in c++17 which is only four years ago and it took most implementations quite a while to implement it in the official <filesystem> location (vs as an experimental header).

Oh. And we’re still lacking any kind of sockets support. There’s this thing I’m hearing about called the Internet that’s going to be big any day now and might need this.

Re:Fix the unit test and open a giant hole everywhere by mgaunard on Saturday December 25, 2021 @12:12PM (#29684863)
I fail to see how "any kind of sockets support" is true, given that there is a networking TS as well as well-established networking libraries.

The main reason why all these things haven't really been integrated into the main language yet is because the committee is still experimenting with various approaches to asynchronous programming and scheduling with goals of also integrating these with heterogeneous computing.

Also sockets are arguably an outdated networking model anyway, so people really serious about high-performance networking will be doing their own thing regardless.

Re:Fix the unit test and open a giant hole everywhere by vlovich123 on Saturday December 25, 2021 @12:43PM (#29685138)
I could buy that reasoning in the 90s. Heck even in 2000 after the dotcom bubble burst. I could maybe buy it in 2010 although by then networking was in no way even remotely niche. We’re now into the third decade of networking and far less important additions to the standard library have been made.

A TS is almost nothing in the c++ world. Each implementation will have its own quirks and unreliable cross platform behavior if it’s even available. There are well established filesystem libraries too (heck the STL implementation is almost identical to boost for that reason to make it easier for those people to port and for the library maintainers you basically just wholesale copy an implementation or at least have a reference one to test most things against).

> Also sockets are arguably an outdated networking model anyway, so people really serious about high-performance networking will be doing their own thing regardless.

I’m eager to hear what you mean by this because I’m unaware of any OS APIs that do Networking in any other way. Unless you’re talking about userspace drivers or DPDK, neither of which see serious adoption outside of data centers (if even there). 99.99% of an networking code I’ve observed across mobile and cloud is socket-based.

Re:Fix the unit test and open a giant hole everywhere by mgaunard on Saturday December 25, 2021 @01:27PM (#29685610)
High-performance networking somewhat excludes mobile and cloud.

Regardless, the io_uring interface, which is general-purpose and still based on file descriptors for its initialization, does go out of its way to work around some of the problems with the socket model.

Re:Fix the unit test and open a giant hole everywhere by fivea on Saturday December 25, 2021 @11:38AM (#29684596)
> Oh. And we’re still lacking any kind of sockets support. There’s this thing I’m hearing about called the Internet that’s going to be big any day now and might need this.

Between libcurl, POCO, or even Qt, does it matter at all that the C++ standard offers "any kind of sockets support"?

Just because Java forced everything under the sun into their standard library, that does not mean that move makes any sense at all. Between Boost and Apache and POCO and others like that, it's high time we stop insisting in this nonsense.

Re:Fix the unit test and open a giant hole everywhere by vlovich123 on Saturday December 25, 2021 @12:32PM (#29685044)
That’s one position. The other is one most other languages choose which is some kind of cross platform sockets support out of the box with packages filling in more niche cases for performance or usability reasons.

You could easily make the same counter argument against including <filesystem> and yet there’s very real value to having something cross platform out of the box that handles elegantly various pitfalls that people typically fall into.

Re:Fix the unit test and open a giant hole everywhere by fivea on Sunday December 26, 2021 @06:18AM (#29691481)
> The other is one most other languages choose which is some kind of cross platform sockets support out of the box with packages filling in more niche cases for performance or usability reasons.

Given that Java followed that path and already acknowledged that mistake by deprecating the original HTTP client, why do people insist in not learning lessons?

Re:Fix the unit test and open a giant hole everywhere by vlovich123 on Sunday December 26, 2021 @10:47AM (#29693312)
HTTP != sockets. I’m not arguing for HTTP within the standard library but for low-level networking support. That one should be non controversial.

As for HTTP within the standard library, I would probably tend to agree although I’d note that Python does have it in its standard module set and I don’t think there’s an acknowledgement that that’s somehow a mistake. Each language fills a different need so framing it as “people insisting in not learning lessons” isn’t helpful. Not to mention that the best way to learn is to experiment and double check if what was once true remains that way (experts know when and in what way to break rules).

Re:Fix the unit test and open a giant hole everywhere by pjmlp on Sunday December 26, 2021 @10:38AM (#29693218)
Because what the standard library offers, is supposed to be available across all platforms and implementations of a specific language, with a certain expectation level in quality.

What is available as external library has various levels of support quality.

That is why we have POSIX, the C runtime that ISO C didn't want to rely on, yet almost every C program uses a part of it.

Re:Fix the unit test and open a giant hole everywhere by jstimpfle on Sunday December 26, 2021 @11:26AM (#29693718)
Pretty sure many or most Windows programs don't use the POSIX library.
Re:Fix the unit test and open a giant hole everywhere by pjmlp on Sunday December 26, 2021 @11:36AM (#29693824)
Indeed, there is a semantic difference between almost every and all of them.

Still it was deemed important enough to spend development resources on the POSIX subsystem to make DoD happy, then SFU and now integrate the Linux kernel.

Re:Fix the unit test and open a giant hole everywhere by jstimpfle on Monday December 27, 2021 @09:50AM (#29702890)
I don't think there is a need to bring up "semantics" here, and the only reasonably defined semantics that I know for the term "almost all" would be from set theory where that is a useless term without an infinite set to measure against.

Common sense anyway would suggest that "almost every" is distinct from "many, but also many not", and the latter is what I think would be a better fit here.

The intention behind my comment (and I thought it was clear without any "semantics" discussion required) was to show that there is no need for the POSIX layer to be included as part of ISO C. POSIX is just a "platform layer" (for all I know), meaning that it is much closer to a "library" than a "runtime". I don't see a need to bundle it tightly with the language. Heck, why did I even bring up Windows as a counterexample, when there are probably a lot of more-or-less ISO C conforming compilers for microcontroller platforms that could never support POSIX.

Re:Fix the unit test and open a giant hole everywhere by fivea on Saturday December 25, 2021 @05:30AM (#29682048)
> Because C stopped evolving long ago (...)

That excuse isn't credible. I mean, I asked about a libraries that handled that usecase, not a standard library. If there was demand for that use case, and that demand popped up so often that it justified the adoption of a de-facto standard, why isn't there any third-party library specialized in that usecase? I mean, putting together a library takes far less work than presenting a proposal to the C standardization committee, which is open to proposals.

So, why isn't there one? Might it be that there is actually no relevant demand for it?

Re:Fix the unit test and open a giant hole everywhere by mgaunard on Saturday December 25, 2021 @06:13AM (#29682288)
How would such a trivial library achieve popularity? It would have to be part of a larger framework.

Anyway I just checked and Glib does seem to have it for example.

Re:Fix the unit test and open a giant hole everywhere by fivea on Saturday December 25, 2021 @06:16AM (#29682305)
> How would such a trivial library achieve popularity?

There is no size limit on how many features could ship with a library. In fact, we live in an age where packages like rimraff report around 50 million downloads per week.

But the original question, and the point it made,still stand: why isn't there such a library to provide such a feature? Might it be that you're overblowing it's demand and usefulness? Perhaps there is no such thing in the standard library because no one bothers with it, or feels it's missing?

Re:Fix the unit test and open a giant hole everywhere by mgaunard on Saturday December 25, 2021 @07:24AM (#29682716)
I already gave you an example of a C library that provided that functionality.

And no, people don't reuse trivial libraries in C, this is not JavaScript.

Re:Fix the unit test and open a giant hole everywhere by fivea on Saturday December 25, 2021 @11:59AM (#29684757)
> And no, people don't reuse trivial libraries in C, this is not JavaScript.

You do realize that tribalist arguments based on absolutely no technical point whatsoever do not make any sense at all, don't you? I mean, who in their right mind would argue that only nodejs developers need to delete dirs?

And if you believe that putting together a reliable cross-platform library that handles idempotent directory creation then be my guest and just do it. How hard would it be, right?

Re:Fix the unit test and open a giant hole everywhere by mgaunard on Saturday December 25, 2021 @12:16PM (#29684900)
This is merely a factual comment.

Node.JS is widely known for having people depend on trivial packages (I vaguely remember some story about half the world being affected because the package doing a trim space went down).

The same is not true for C. Libraries there tend to cover larger domains.

Whether that is caused by technical limitations, culture, or competence of the developers is all irrelevant to that statement.

Re:Fix the unit test and open a giant hole everywhere by rat9988 on Saturday December 25, 2021 @09:31AM (#29683593)
Cross plateform make directory is trivial?
Re:Fix the unit test and open a giant hole everywhere by unionpivo on Saturday December 25, 2021 @07:05AM (#29682599)
Libraries are a pain in C. So most C programs use less of them, than if you are writing js
Re:Fix the unit test and open a giant hole everywhere by fivea on Saturday December 25, 2021 @07:13AM (#29682651)
> Libraries are a pain in C.

They really aren't. At all. What exactly are the pain points you experienced with C libraries?

> So most C programs use less of them

This assertion doesn't hold any water. In all the years I worked with C and C++ projects, not even once did I ever heard "let's not add a library, it's hard".

Do you actually have a concrete, real issue in mind?

Re:Fix the unit test and open a giant hole everywhere by cowsandmilk on Saturday December 25, 2021 @08:29AM (#29683185)
> What exactly are the pain points you experienced with C libraries?

1. Varying build systems. Does the dependency use cmake? Autoconf? A shell script where you’re instructed to modify some variables specific to your system?

2. The above, but my software is shipping on Linux, windows, and Mac. What’s the best way to get this dependency compiled? Does upstream even support all these OSes?

In all my years of doing C programming, I’ve never seen some add a single function library like is_even and then another like is_odd. But you do see it in languages where it is really easy to add libraries.

Re:Fix the unit test and open a giant hole everywhere by convolvatron on Saturday December 25, 2021 @12:07PM (#29684820)
in C I always end up building my own world because the default one is broken. for example null terminated strings are just really awful to work with. i'm not alone - alot of larger C projects have their own runtime, particularly memory management. another cross-cutting concern is multihreaded safety and scheduling.

unless the library author broke those functions out, then I need to rehost it in my environment. so I usually end up forking any library that I want to use. this isn't all bad - I find some bugs and integrate the build. I adopt the library and develop and understanding of it sufficient to maintain it myself.

contrast that to a language environment where those things - strings and threads and build are all standardized.

Re:Fix the unit test and open a giant hole everywhere by mgaunard on Saturday December 25, 2021 @01:35PM (#29685689)
My point of view is that if you program in C you most likely care about being in control of what happens.

Your approach makes perfect sense within that context.

Re:Fix the unit test and open a giant hole everywhere by infotogivenm on Saturday December 25, 2021 @01:07AM (#29680904)
There’s absolutely a middle ground, and you are right: it is fork and exec with discrete arguments.

It is the intermediary subshell in system() that is the root of all evil, not composing unix programs together; that is what our OS forefathers intended.

Re:Fix the unit test and open a giant hole everywhere by theptip on Saturday December 25, 2021 @02:38AM (#29681274)
I agree. Starting a subprocess is fine. Using an existing tool (mkdir) was probably a better approach than reimplementing it.

The error: you should never pass unescaped/untrusted input into a subshell.

In Python, it’s the difference between the safe:

    def make_dir(path):
      subprocess.run([“mkdir”, “-p”, path])
Instead of:

    def make_dir_unsafe(path):
      subprocess.run(“mkdir -p “ + path, shell=True)
The latter is easier to type because you don’t need to use the annoying list syntax, but it’s unsafe. If you are writing a library function a subshell is not a sound approach.
Re:Fix the unit test and open a giant hole everywhere by blorgle on Saturday December 25, 2021 @04:09AM (#29681681)
You shouldn't pass unescaped/untrusted input into a subshell but your suggestion doesn't solve for every case. It protects against ";" but not path traversal with "../../../../../foo/bar".
Re:Fix the unit test and open a giant hole everywhere by yjftsjthsd-h on Saturday December 25, 2021 @11:15AM (#29684419)
You'd have to cover that if you wrote your own library function, though; it's attack surface, but not uniquely so.
Re:Fix the unit test and open a giant hole everywhere by Noe2097 on Saturday December 25, 2021 @05:09AM (#29681955)
Hmmm. In Python, I would leverage the dedicated utility of the standard library:

  os.makedirs(path)
Re:Fix the unit test and open a giant hole everywhere by theptip on Saturday December 25, 2021 @03:10PM (#29686524)
Yes, agreed. I was using Python code to more explicitly illustrate the subprocess vs shell distinction in the general case, not to provide a solution to the specific problem of making a directory.
Re:Fix the unit test and open a giant hole everywhere by im3w1l on Saturday December 25, 2021 @04:41AM (#29681838)

    subprocess.run([“mkdir”, “-p”, path])
This does not work for all values of path. And I'm not just referring to blorgle's complaint. I'm being deliberately vague to make you pause for a second and think.
Re:Fix the unit test and open a giant hole everywhere by ynik on Saturday December 25, 2021 @06:48AM (#29682494)
You seem to be getting downvoted for being too vague, so I will post the solution:

The snippet breaks for paths starting with a minus, as mkdir will interpret those as another option.

The solution is:

    subprocess.run(["mkdir", "-p", "--", path])
Re:Fix the unit test and open a giant hole everywhere by Kinrany on Saturday December 25, 2021 @05:40AM (#29682096)
Does `path = "$(rm -rf /)"` work in this context?
Re:Fix the unit test and open a giant hole everywhere by bstpierre on Saturday December 25, 2021 @07:24AM (#29682717)
No, because this isn’t being run through the shell, it’s directly exec’ing the program. So you’re just going to get a dir named “$(rm -rf /)”.
Re:Fix the unit test and open a giant hole everywhere by im3w1l on Saturday December 25, 2021 @07:33AM (#29682765)
It's funny how easy it is to miss characters with special meaning. In this case it's the slash. You would get a directory named "$(rm -rf " with a subdirectory named ")"
Re:Fix the unit test and open a giant hole everywhere by pabs3 on Saturday December 25, 2021 @03:23AM (#29681470)
I like to do this to avoid list syntax:

    def make_dir(path):
      subprocess.run('mkdir -p'.split() + [path])
Re:Fix the unit test and open a giant hole everywhere by loloquwowndueo on Saturday December 25, 2021 @07:14AM (#29682659)
So you like typing more and making your program harder to read to avoid using two apostrophes and two commas.
Re:Fix the unit test and open a giant hole everywhere by cowsandmilk on Saturday December 25, 2021 @08:37AM (#29683255)
If you have a command with lots of arguments, parent’s approach makes a lot of sense. 20 arguments that you can copy out of the shell into your program. And when you want to make changes and test via the shell, your copy and paste is also easy.

I’ve seen bugs where some forgot to add quotes and commas to split some arguments and the program did the wrong thing. Parent’s approach avoids that. However, things like spaces in quoted strings are not correctly handled.

Re:Fix the unit test and open a giant hole everywhere by loloquwowndueo on Saturday December 25, 2021 @11:15AM (#29684416)
Naively splitting commands using split() will bite you in the ass eventually. Use shlex.split as the other comment says, or just stop being lazy and craft the argument list carefully, once, and be done with it. Shortcuts here aren’t worth the risk.
Re:Fix the unit test and open a giant hole everywhere by div72 on Saturday December 25, 2021 @09:46AM (#29683712)
I am not exactly sure if parent's code execute the way you think it does. path is provided as an argument to mkdir -p, even if you set path to be a string with 20 arguments copied from your shell, it'll get passed as one.

I assume you meant something like subprocess.run(['mkdir', '-p', '--', *path.split()]), which in that case using shlex.split will fix your problems with quoted strings.

Re:Fix the unit test and open a giant hole everywhere by me_again on Saturday December 25, 2021 @10:47AM (#29684182)
I'd want a better reason before shelling out personally. Apart from security, other issues are:

- performance. Obviously that depends on the circumstances but if you are working on a function that is used in many places you don't know the circumstances.

- error handling and reporting. Are you going to capture stderr from mkdir? If this fails can you provide a clear indication of the problem to the caller?

- paths. If you run mkdir without specifying an absolute path your program may fail if $path is not what you expect. That can also be a vulnerability.

- dependencies. Now you won't run in a distroless container or other cut down environment

- you lose type safety

Not saying "never shell out" but I've been burned by most of these so I am cautious about it.

Re:Fix the unit test and open a giant hole everywhere by hamburglar on Saturday December 25, 2021 @12:21PM (#29684946)
“Never shell out” is pretty close to correct though. I’d say “only shell out as a last resort, and only after you’ve considered all the drawbacks.”
Re:Fix the unit test and open a giant hole everywhere by ptx on Saturday December 25, 2021 @09:18AM (#29683504)
> It is the intermediary subshell in system() that is the root of all evil

Certainly the worst of it, but even if you pass the untrusted data as a discrete arguments you're still vulnerable to parameter injection.

Let's say, for example, that you're running ["find", USER_SUPPLIED_PATH, "-type", "f"]. If the user supplies "-delete" as the path, the resulting behaviour would not be as intended.

Re:Fix the unit test and open a giant hole everywhere by infotogivenm on Saturday December 25, 2021 @02:07PM (#29685996)
Very true. A pro tip here is use ‘—-‘ :)
Re:Fix the unit test and open a giant hole everywhere by elurg on Saturday December 25, 2021 @03:41AM (#29681542)
Doing fork/exec for the mkdir tool is still many times slower that calling a good library function.
Re:Fix the unit test and open a giant hole everywhere by fivea on Saturday December 25, 2021 @03:48AM (#29681583)
> Doing fork/exec for the mkdir tool is still many times slower that calling a good library function.

Would it be noticeably slower to the point it's measurably relevant, though? Or are we talking about something well within the domain of premature optimization?

I mean,if you're doing IO, how often does creating a dir end up being the performance bottleneck?

Re:Fix the unit test and open a giant hole everywhere by thebackup on Saturday December 25, 2021 @04:43AM (#29681853)
Premature optimization is a common disease amongst C and C++ devs. I often come across code that is written in an obscure/complex way just because someone thought execution speed triumphs code readability. In most cases (around 9 times out of 10) it is on a non-time-critical path of the code and the developer who wrote it didn't even bother to run a profiler on it. My work would be significantly easier if we could learn to prioritize code readability.
Re:Fix the unit test and open a giant hole everywhere by mgaunard on Saturday December 25, 2021 @05:10AM (#29681962)
The point of those languages is to make it explicit when you're doing ridiculously costly things so that you're forced to do a reasonable implementation instead.
Re:Fix the unit test and open a giant hole everywhere by klyrs on Saturday December 25, 2021 @11:05AM (#29684327)
No, any language that allows operator overloading can hide ridiculously costly operations behind mundane-looking code. C is pretty good about that, but C++ is not: you can even overload the comma operator!
Re:Fix the unit test and open a giant hole everywhere by mgaunard on Saturday December 25, 2021 @01:52PM (#29685843)
Consider a + b + c.

When you read the code, you know what the types of a, b and c are. You know whether they're built-in types, and therefore whether those operator expressions are actually function calls.

Nothing is hidden, so long as the programmer doesn't make invalid assumptions based on his experience with other, different languages.

Re:Fix the unit test and open a giant hole everywhere by klyrs on Saturday December 25, 2021 @02:19PM (#29686102)
Your point appears to be "if you know the entire codebase, nothing is hidden." This is tautologically true of all languages. If you're accustomed to working on small projects, then that attitude will work out for you. If you work on large codebases, it isn't always so easy, especially if folks use a lot of template magic.

In your case, if a, b, and c are all the same type, then b+c can return a completely different type, and you'll need to track that down before you know what type a+(b+c) will have. And in that case, (a+b)+c can have a different type from a+(b+c). If you've memorized the associativity rules for the language, you'll know what to expect -- but it's not at all obvious from the notation.

And, sure. If you know absolutely everything about the language and everything about the codebase, you can make accurate predictions about the behavior of any given line. But if you're coming into a new codebase, there's no way to know at a glance what a given line of code is going to cost. That's what folks are calling hidden complexity.

Oh -- and thanks to the auto keyword, you don't always know the types of a, b, and c without hunting things down.

Re:Fix the unit test and open a giant hole everywhere by mgaunard on Saturday December 25, 2021 @02:42PM (#29686298)
You don't need to know the whole codebase to understand a piece of code, just what the declarations of the names being referenced in that piece of code are (in C++, a name may be a macro, a type, a template, a variable or a function, and depending on which kind, may have all kinds of other properties attached).

Yes, you need to look at the declarations of names to know what the resulting type of expressions involving those names is. I don't see that as being surprising or unusual. The same is also true in C.

Re:Fix the unit test and open a giant hole everywhere by tylerhou on Saturday December 25, 2021 @05:27AM (#29682035)
I disagree that C++ makes expensive operations explicit; see the copy constructor and the copy assignment operators. C, maybe.
Re:Fix the unit test and open a giant hole everywhere by mgaunard on Saturday December 25, 2021 @06:09AM (#29682261)
A C++ developer would know what an assignment or copy entails and would spot them on sight.
Re:Fix the unit test and open a giant hole everywhere by tylerhou on Saturday December 25, 2021 @07:21AM (#29682698)
It's impossible to tell whether a specific line of code makes a copy without knowing the types of all values. i.e.

    std::vector<int> numbers = Func();
may or may not make a copy depending on the signature of Func. So it's not clear at the callsite how expensive that line of code is. This is unlike Go (which has this explicitly as philosophy) where you need to call a copy function/use append to make a copy of a slice.
Re:Fix the unit test and open a giant hole everywhere by mgaunard on Saturday December 25, 2021 @07:29AM (#29682745)
Knowing the declarations of the various names a piece of code uses is of course a requirement in order to understand what it does.

In that example, there would always be a move or copy there, though there are scenarios in which it might be elided.

Re:Fix the unit test and open a giant hole everywhere by hamburglar on Saturday December 25, 2021 @12:37PM (#29685084)
It depends a lot on how often you’re trying to do it and the conditions you’re doing it under. I’ve worked on a couple of projects where I own an agent process that runs on thousands of machines that are not mine (so I have no control of whether they are overloaded or in some pathological state), some of which are bumping up against memory and/or cpu and or IO limits, and a single blocking system call is many orders of magnitude more practical than the overhead of fork/exec and the dozens or hundreds of blocking system calls and memory allocations that entails.

This is, of course, an extreme example. But there are definitely cases where it matters a lot.

Re:Fix the unit test and open a giant hole everywhere by infotogivenm on Saturday December 25, 2021 @02:11PM (#29686032)
Completely agreed. But likely many many times faster to write. Just depends what youre optimizing for.
Re:Fix the unit test and open a giant hole everywhere by taneq on Saturday December 25, 2021 @09:28AM (#29683570)
I think the real issue here is the lack of distinction in the rest of the codebase between trusted and untrusted input. There should be only a few, clearly identified functions that you do pass unsanitised user input into.
Re:Fix the unit test and open a giant hole everywhere by horsawlarway on Saturday December 25, 2021 @02:58PM (#29686420)
I think this, really, is the take away. Everyone is getting stuck on the subshell part, but it's the untrusted data part that's the problem.

Especially because the early examples in the article (making subdirs for something like different packages/release - "project/staging", "project/production") are almost certainly safe to use a subshell for - so much so that I was genuinely wondering why the author wouldn't just call out to "mkdir -p" for that use case.

The error came when someone decided to pass untrusted/user-supplied data to this function (the later example - usernames).

But I think doing the later is a huge mistake either way. Don't pass untrusted & unsanitized user data ANYWHERE that isn't explicitly designed for it.

Who gives a hoot whether that's to a subshell, or a pipe/stream, or to a sql query, or your DOM, or any other of hundred of places that user supplied data isn't safe.

Re:Fix the unit test and open a giant hole everywhere by selfhoster11 on Saturday December 25, 2021 @08:33PM (#29688845)
I like the idea that Joel had: make untrusted user provided strings into a distinct data type that doesn't easily fit into the mold of a trusted, already-sanitised string type.

If you ever see an unsafe string usage being passed into a function that expects a safe string, without an accompanying call to sanitise(), that's very wrong (and won't work if the language is strongly typed) and should be recognised/corrected as such immediately.

Re:Fix the unit test and open a giant hole everywhere by taneq on Sunday December 26, 2021 @07:30AM (#29691897)
Yeah, I actually nearly put in something about separate trusted vs. untrusted types but didn't want to dilute the main point or hijack it religious wars over typed languages. :P
Re:Fix the unit test and open a giant hole everywhere by selfhoster11 on Sunday December 26, 2021 @08:15AM (#29692147)
I think there could be some ways to make it work even in untyped/dynamically typed languages, as long as they support prototypes. Or use Applications Hungarian (not Systems Hungarian!) to add this facet of type information.
Re:Fix the unit test and open a giant hole everywhere by ufmace on Saturday December 25, 2021 @01:09AM (#29680910)
It seems to me that hiding a risky implementation is the problem. If you were directly shelling out to `mkdir -p` yourself, it would (hopefully) at least be obvious that it's a risky thing to do, and you need to make sure that either the input couldn't possibly contain any data touched by a malicious user or that it was sufficiently well sanitized to protect against that sort of thing. But if it's hidden behind a friendly-look library call, it would be less obvious that it's a risk, and it's easier to see someone absent-mindedly calling it because it's handy and passing in data from an unauthenticated web user or something.
Re:Fix the unit test and open a giant hole everywhere by newaccount74 on Saturday December 25, 2021 @12:50PM (#29685213)
I feel strongly that you shouldn't go looking for external code that does simple things like this. You can reimplement the relevant parts of mkdir -p in 20 lines of C code, and I think that's what you should do if the standard library doesn't have it.

Having the code inline makes it easy to audit, and prevents problems when a dependency changes 5 years later.

It also trains people to write proper C code. Writing loops and checking errno should be second nature for a C programmer, and doing it often helps you learn.

Re:Fix the unit test and open a giant hole everywhere by bombcar on Saturday December 25, 2021 @01:04AM (#29680891)
You can do something akin to parameterizing SQL with fork and exec (directly setting the arguments) but at that point you might as well call whatever mkdir is doing.
Re:Fix the unit test and open a giant hole everywhere by shadowgovt on Saturday December 25, 2021 @11:37AM (#29684588)
Yep. Really, the meta-problem here is that someone would want to go run a subprocess because the standard library isn't powerful enough.
Re:Fix the unit test and open a giant hole everywhere by catern on Saturday December 25, 2021 @01:15AM (#29680932)
I think the real fix here is to investigate why you're creating possibly-already-existing directories anyway. Most files or directories should either be owned by a specific component (which then creates them and fails if they already exist) or should be assumed to already exist (as a requirement that the environment is set up properly.
Re:Fix the unit test and open a giant hole everywhere by spc476 on Saturday December 25, 2021 @02:20AM (#29681205)
It depends upon usage. My blogging software keeps entries in the file system, in a hierarchy as YYYY/MM/DD. I just create the directories as I add entries by doing a "mkdir(year); mkdir(year "/" month); mkdir(year "/" month "/" day)".