5 ms·
Pop quiz, which of these is safe, given "char buf[80]" and arbitrary user input in argv[1]? gets(buf); scanf("%s", buf); strcpy(buf, argv[1]);
by nneonneo 4y ago
Pop quiz, which of these is safe, given "char buf[80]" and arbitrary user input in argv[1]?
gets(buf);
scanf("%s", buf);
strcpy(buf, argv[1]);
scanf("%80s", buf);
strncpy(buf, argv[1], 80);
snprintf(buf, 80, argv[1]);
----
The delightful answer is none of them. The first three have no bounds checking at all, meaning that they will happily overflow the buffer to an arbitrary extent (gets, at least, will usually trigger a warning on modern compilers). The next two have off-by-one errors: scanf will write a NUL byte out of bounds (and that's exploitable! https://googleprojectzero.blogspot.com/2014/08/the-poisoned-nul-byte-2014-edition.html https://googleprojectzero.blogspot.com/2014/08/the-poisoned-...) while strncpy will fail to NUL-terminate the string. The last one uses the right buffer length, but treats user input as a format string and can leak memory contents or produce arbitrary memory corruption with the %n format specifier.
C string handling practically invites off-by-one errors and horrible security practices out-of-the-box.
- cdelsolar 4y agoSo then what do you do?
- myrmidon 4y agoAre you sure that strncpy does an out-of-bound write here? I believe it doesnt, but would give you an unterminated string in buf which is also... less than ideal (if the input is 80 non-null characters or longer).
- nneonneo 4y agoUgh, I had that in my comment before I "refactored" it. Fixed now, thanks for pointing it out.
- Lt_Riza_Hawkeye 4y agoYeah, and I wouldn't say it's definitely unsafe. You can memchr a '\0' out of it (or not) to determine if a null terminator got in there or not.
- nneonneo 4y agoI mean, you could but a lot of people do not. Plus, strncpy is terribly inefficient if the source string is tiny, because it’ll fill the rest of the buffer with NULs.
- myrmidon 4y agoOne mans inefficiency is another mans resistance to timing attacks :P (this is tongue-in-cheek, the function is still bad IMO because it almost never does what you need it to. If it guaranteed null termination it would be more useful)
- throwaway2037 4y agoThis is a very thoughtful post. Real question: Is the solution to avoid C strings entirely? Use something like a Pascal string that includes the length?
- jiggawatts 4y agoYes. For the love of god, a thousand times yes. Every other language, even some of the "C-compatible" ones uses explicit-length strings.
- fractalb 4y agosnprintf(buf, 80, “%s”, argv[1]); Should work.
- LoganDark 4y agoAs long as you replace the 'smart'-quotes with actual quotes. -Emily
- alxmdev 4y agoBingo, you should never pass arbitrary strings where they could be used as format specifiers, it's like running arbitrary code. Some compilers even issue warnings when you pass non-literal format strings to the printf family.
- robomartin 4y ago> The delightful answer is none of them. No. Sorry. This is bad programming. C'mon. I started programming back in the 8080, 8085, 6502, etc. days. I had to program some prototype computers using a hex keypad while entering raw machine code (not even assembler). I still own a couple of these: https://i.imgur.com/ZsIJj1p.png https://i.imgur.com/ZsIJj1p.png In a couple of cases I had to take this approach to bootstrap Forth on a 6502, then write a full Forth code editor and finally write the robotics application from there. Do not confuse bad programming or lack of knowledge with something attributable to a language, any language. A knowledgeable software developer, among other things, stays clear of these issues. This is also the value of experience and exposure to a wide range of technologies. It's like blaming MicroPython for a machine getting destroyed because garbage collection interrupted a critical real time process. There's nothing wrong with MicroPython in that regard, the programmer/designer of the embedded system either lacked knowledge and understanding. Part of the problem, as I see it, is that a good deal of modern university CS degrees don't even touch low level stuff. They start students on languages like Javascript and Python. These are fantastic, however, someone with deep-rooted experience in these languages who jumps into C is very likely to do some truly horrific things. The language isn't the problem, at all. I mean, not to go too far, the Linux kernel is written in C. Right? It's about the person, not the language.
- skribanto 4y agoOne could take a glance at these and easily believe that they do the right thing. I don’t think that no one would accidentally miss such a small error from time to time.
- robomartin 4y agoOf course, and the more experience you have the less of this will happen. I've been writing software in over a dozen languages for over 30 years. Generally speaking, when I write code, even complex code, in any language, it just works. Not because I am something special. I have done a lot of of work across a wide range of application domains and have made my share of mistakes over the years. Of course I make mistakes. Everyone does. Yet these mistakes. They have nothing to do with lack of domain knowledge. People who approach C without having a clue as to how memory, registers and the internals of a processor and memory system work are going to create bad code. Blaming the language, the tools, is irrational. You can write perfectly good, safe and performant code in assembler. And boy, can assembler be a minefield in the hands of someone without experience! Are we going to blame the processor microcode then? No, of course not.
- up2isomorphism 4y agoDude, your last one is not even the correct way of calling snprintf, what the hell are you talking about?
- nneonneo 4y agoI mean, none of these are correct. But they will all compile, often with no warnings at all. The format string doesn’t need to be a literal string: this is useful for situations like localization.
- KyeRussell 4y agoNo surprise that this completely unjustified vitriolic reaction comes from someone arguing about C.
- heywhatupboys 4y ago> snprintf(buf, 80, argv[1]); yeaaa, this has much bigger problems than a null write...
- Woodi 4y ago> C string handling practically invites off-by-one errors and horrible security practices out-of-the-box. At that point it is beating dead horse - it is such well known fact how C strings works. And it's insanity that noone proposed new standard library with better implementation for strings domain. Boo hoo "old programs" bla bla... And it's total insanity to blame powerfull language for allowing you to do almost anything in it. You don't even need to ask commitee for permission to roll your own _low level constructs_ - how insane is that ? ;) But keep spitting on what gives you freedom...