3 ms·

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 iden

by taneq 5y ago

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.

horsawlarway 5y ago
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.
selfhoster11 5y ago
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.
- taneq 5y ago
  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
  - selfhoster11 5y ago
    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.

by taneq 5y ago

horsawlarway 5y ago

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.

selfhoster11 5y ago

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.

taneq 5y ago
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
- selfhoster11 5y ago
  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.