I wonder if this bug in logic (instead of buffer overflows) would also have been less likely in a different language. Would it have been more obvious in a language where it's easier to work with dynamically allocated arrays and strings?
With my Rust hat on: I don't think that Rust would have solved this. It might have made the code in question easier to understand, as you note, but this kind of error can still happen in any language.
Looking at the patch[1], probably not. There isn't really a lot of complex string handling involved; it's basically just forgetting to forbid "--". I don't really see how any language choice could help you with this.
It shouldn't have to forbid that. The editor and privileged files shouldn't be in the same string. It should just be appending the list of temporary files to the editor command, and running that unprivileged.
You don't even need a temporary file; opening the file directly in sudoedit then passing /dev/fd/N to the spawned edit process after dropping privileges would work (a-la capabilities). But sudoedit being implemented in terms of sudo makes it hard.
edit: apparently things are more complex and sudoedit already runs the command unprivileged; the attack is in filename expansion in sudoedit itself.
Besides the fact that that wouldn't actually have prevented this bug (which you acknowledge in your edit), /dev/fd/N is a linuxism, so wouldn't work on other unices. And has slightly different semantics than the current implementation, where your changes to the file aren't actually updated in the original privileged file until after you exit the editor.
I would say a more type-oriented mentality would make this kind of bug less likely; thinking of -- as a magic value rather than a different kind of thing from a regular argument makes it easy to forget the distinction, and a mentality where you're "sanitizing a string" is far less reliable than one where you're transforming between two distinct formats.
Good point. A "parse, don't validate" approach to handling the contents of `$EDITOR` would look like `parseEditorEnv :: String -> Maybe ProgramPath`, and that parse should fail.
But that's the wrong thing. The problem is not that EDITOR must be a program, but (apparently) that they're parsing the expansion of EDITOR (along with other stuff) to figure out what file to operate on.
I don't see a change to language, per se, that would have helped, really.
A system with more of an object capabilities model could have helped, though. The goal wasn't really "let the user run their editor as root (when they ask for it)", but "let the user work with this particular file from their editor (when they ask for it)".
On reflection, I think C is a language where this kind of error is less likely. In languages where it's easy to cram strings together and parse the results, people are more likely to do it. Those things are a bit of a pain in C, so people are more likely to do things another (and in this case better) way. Of course, that was obviously no guarantee.
