Microsoft finds critical hole in operating system that for once isn't Windows Microsoft has described a severe ChromeOS security vulnerability that one of its researchers reported to Google in late April. The bug was promptly fixed and, about a month later, merged in ChromeOS code then released on June 15, 2022 and detailed by Redmond in a report released on Friday. Microsoft's write-up is noteworthy …
They could have even kept the strcpy() as is after adding the missing length check.
For extra measure the actual patch adds the missing length check and switches to strncpy() which doesn't allow buffer overflow, so they implemented technically a double length check to the code.
The strcpy() is not always unsafe but it has a poor name which makes some developers to assume it has something to do with copying strings. It's actually a function to copy range of memory when given two pointers and it will keep going until it sees a zero byte in the source memory area. It's often highly optimized for this special behavior so it should be used if and only if the behavior you need is exactly that.
strlcpy/strlcat will silently truncate the copied string. Sometimes, that's what you want, but I find that usually, it indicates a problem. You basically need two functions : one says "truncation is expected in this case", vs. "truncation means something went wrong; stop now and emit a hopefully helpful message."
After years of trying various solutions, I now have supplemented strlcpy/strlcat with 'error' versions. With these, if the string doesn't fit into the specified buffer size, you get a message and an assert() is triggered.
Similarly, I have an snprintf_err() function for cases where a buffer overflow should be a red flag.
In theory, these could just be replaced by the non-_err() functions on non-debug builds. However, 99% of these calls are not in high-performance code. For the remaining 1%, I would use
#ifdef DEBUG
strlcpy_err( arguments)
#else
strlcpy( arguments) or strcpy( arguments)
#endif
I've been writing C code since 1987, and have been slow to the party on these points. Kinda wish I could go back in time and tell my younger self about the value of adding in these "idiot checks". I don't think I've had security issues as a result, but Dog knows I've certainly had "code-not-working" issues.
Why endless libraries and APIs area bad : you cannot possibly be familiar with every library to that level of detail.
Though in the case of the standard C library in op systems, using it with a lack of familiarity is less excusable.
On the other hand, mistake identified; changed and checked is a good thing.
Which is why people who care about code correctness will use gcc -Wall to enable all warnings, and get their code to compile cleanly or if some warnings remain insure those warnings are innocuous.
If you do that you won't need to know this option has been added, you'd just see its practical effect when you compile code using strcpy() with -Wall.
Good, but incomplete advice. Unless a fit of sanity has hit gcc developers since I stopped caring, to really
catch _ALL_ warning instances, you have (had?) do use
-Wall -ansi -pedantic
and of course -Werror
Apparently someone quite important considered standard compliance to be of lesser importance than continuing to "honor" a bug left over from some versions of the (pre-standard) Portable C Compiler.
In short (and invitation to the rabbit hole) the result of a pointer cast is not a valid modifiable lvalue
(I think I have that wording right)
Int myint;
char *myptr;
[...]
myint = *((int *)myptr)++; /* modulo some paren juggling */
[...]
Not that the gcc folks are alone in this. A very respected programming columnist lost some of my respect for flaming a compiler for having the audacity to issue a warning for this construct.
Yes,I do know that strcpy CAN be used safely if all the input parameters are validated, but why tempt fate? Any static code analyzer should have flagged it. I read the MSFT report, and it wasn't clear how strcpy crept in. It should have been caught eons ago. I have known of hackers introducing specific bugs like this.
Are these developers unable to use version control system?
It's clear that the vulnerability was caused by this code which is obviously bad:
https://chromium.googlesource.com/chromiumos/third_party/adhd/+/660ae6c9ef3781b9b631131f9c1aa654c73d63a9%5E%21/#F1
And it still has reviewed-by tags so supposedly it was reviewed by another developer. Either both were incompetent or or was malicious.
And as you can clearly see from the path, this is some 3rd party Bluetooth service.
One of the major reasons is legacy code. I was involved in the maintenance of a very large retail system that had its roots in MS-DOS.
There were still large parts that used strcpy strcmp etc. No one wanted to refactor those as they worked. However we did have warnings maxed with warnings as errors and just added a define around each call to hide that specific warning.
The other rule was if you had to make any changes in that area which meant it would be tested you were to change the strcpy/strcmp etc. The preferred fix was to replace the char * with a std::string or CString.
Article suggests that MS submitted the finding with a bit of giddiness that they found something in ChromeOS's code and could take Google down a peg or two. HA they used strcopy!?!?
The thing is, all those bugs that Google found in MS's stuff were found without the source code. Imagine the trove of bugs google engineers would find if they had Windows code at their disposal.
And the fact that this MS engineer first found the use of strcopy, then thought hey that's not secure, I bet I can exploit that, only goes to highlight the advantage of an Open source OS over something closed like Windows... Security researchers can simply search the code for insecure techniques and get them corrected. While closed source stuff could have unreported exploits all over with no reports.
I hope the victorious attitude suggested by the author was his own spin, as MS certainly has nothing to pat themselves on the back for in this case.
"Article suggests that MS submitted the finding with a bit of giddiness"
Oh dear. Reading what you want to read, I guess.
I saw this as a complimentary article describing how MS researchers responsibly reported a flaw in a widely used platform and Google's timely and effective response. Wouldn't it be nice if everybody played the game that way. Which they often do...
The suggestion that MS might be being triumphal about this is 1) yours and 2) a sub-ed's crack at titling the article in true Reg style, i.e. somewhat tabloidy and tongue in cheek playing on the traditional perceived antipathy between the two sides.
(edit: I try not to play Corp A vs. Corp B / Platform X is better than Platform Y games, it's counter productive. Just give me something that works safely, please)
I read it the same way as the original AC OP, given that Microsoft has done exactly that before - found a vulnerability in someone else's system and then opined about how insecure that system is.
It's great that Microsoft did report it responsibly, and that Google quickly patched it. That's how it should be. Unfortunately, as the article pointed out, that's often not how Microsoft responds when someone reports a vulnerability in Microsoft code.
5 Thou shalt check the array bounds of all strings (indeed, all arrays), for surely where thou typest ``foo'' someone someday shall type ``supercalifragilisticexpialidocious''.
We have known for decades that out of bounds access is an issue, which is why strncpy() is the preferred method.
Also see the next commandment
6 If a function be advertised to return an error code in the event of difficulties, thou shalt check for that code, yea, even though the checks triple the size of thy code and produce aches in thy typing fingers, for if thou thinkest ``it cannot happen to me'', the gods shall surely punish thee for thy arrogance.
The "10 Commandments" relies on developers actually RTFM.
Come on now people, how many of you actually do read the frigging manual to anything these days?
The last one that I tried was painful. The 5pt type was agony on my eyes and I had to resort to the old magnifying glass. Even then...
As for using strcpy in 2022? Google, you are shite. Go and sit on the naughty step until 2023.
<snippet>
Probably crept in because someone did a copy and paste from Stack Overflow
</snippet>
But in all seriousness, I've seen a lot of poor code around bluetooth drivers and the like over the years. It feels like they are written in a rush by someone with no investment in the result.
>Come on now people, how many of you actually do read the frigging manual to anything these days?
Manuals are so passe. You just do a youTube video complete with some home made visuals and an exciting soundtrack. "Information free" is the new trend. Writing is so last year...
>As for using strcpy in 2022?
Works fine provided you know what you're doing. The critical flaw is using a string for an argument -- its not just grossly inefficient but unless you check carefully for what you're getting (not just length but also form, escape patterns and so on) you're going to get p0wned sooner rather than later.
The alternative would be to use Pascal format strings. I think Microsoft has a type that uses them. These include the string's length at the start of the array of string elements. Obviously you could wrap these up in several layers of objects but it would still be the same fundamental concept -- you either have to bound the data using a delimeter or by an explicit value.
If you're a newbie to writing languages that use C-similar syntax, i.e. C, C++, Java, Pascal and maybe even Perl, you can do a lot worse than to beg, buy, borrow or steal a copy of "The Practice of Programming" by Kernighan and Pike. I wish it, or something like it, had been available when I was learning to write maintainable programs.
The book is well written and contains excellent advice on:
- writing well-documented programs
- choosing informative names for variables, functions, etc
- structuring programs to be both efficient and easy to debug.
Maybe they wouldn't own 99.9% of malware victims on the sieve of an operating system that is windows if they actually bothered to fix their shite, rather than gloating over other people's errors.
The irony of throwing stones and microsoft knows no bounds.
gcc has the #pragma poison directive, which causes it to emit a hard error if it encounters a listed function: example list
And some of the replacements aren't 100% safe either: a comment from Linus the day before yesterday on "a very subtle difference between strlcpy and strscpy".
Exactly. A good compiler setup will typically warn about unsafe function use and _may_ suggest an alternative.
To still be using what are effectively deprecated functions are a pointer to either learning using out-of-date texts or just not learning and understanding how functions actually... function.
strcpy will gallop off into the sunset if there is no terminating \0 in the source buffer, that's its fundamental behaviour and there's no backwards compatible way to make it act differently with the parameters it receives.
Also, if your source data is validated then arguably there's nothing wrong with using it.
"As there are no bounds checks on the user-supplied identity argument before invoking strcpy
No bounds checks - yet again.
The fundamental problem is not that strcpy() is "unsafe" in that it admittedly leaves bounds checking to the programmer, but that nobody seems to recognise the need to use it with proper forethought by taking account of this. There are literally hundreds (if not thousands) of functions and methods in numerous languages that can be hazardous if applied without due attention to security, but that shouldn't brand them as defective. The primary defect is commonly the lack of attention that leads to their unsafe use.
Just as in the non-digital world (where potential hazards also abound) it's our responsibility to make sure we're behaving sensibly when programming.
"nobody seems to recognise the need to use it with proper forethought"
As Tony Hoare famously said, "A programming language designer should be responsible for the mistakes made by programmers using the language." This goes double for mistakes _commonly_ made even by experienced programmers.
"Programmers should be better" is no solution; programming is hard enough already, and no human being can sustain a consistently high degree of vigilance especially when working under time pressure. Good languages - and their supporting tool chains - should take whatever burden then can off of programmers.
(And yes, there are good reasons for why C is the way it is, mainly to do with performance and compactness and the limitations of the tools available at the time.)
"A programming language designer should be responsible for the mistakes made by programmers using the language"
Does that apply to those designing assemblers too? The range of possible programming errors in assembler is probably infinite, so that problem would be pretty intractable.
At some point, the user of any programming language must take responsibility for their own output, just like in any other engineering discipline. Nobody would expect an aeronautics engineer to absolve themselves for (let's say) a space vehicle booster blowing up by saying "but my automated tools didn't show there was a problem" - (that was the argument of management, not the engineers, in the case of both Shuttle incidentst).
Sorry, I forgot, software development isn't an engineering discipline - it's an art, so we can't legitimately expect it to be conducted with safety or security in mind.
"A programming language designer should be responsible for the mistakes made by programmers using the language."
Seriously! Humans have an amazing ability for stupidity. To paraphrase Einstien "the universe is finite, human stupidity is infinite"
The phrases I love hearing are:
"Well it appeared to be a good idea at the time"
"Whoa did not see that coming"
"No one would ever do that/ be that stupid"
"This is only a quick and dirty fix it doesn't need to go into production..."
If strcpy() is so bad, what are people even doing still implementing it in the library? Just leave the function out altogether, so people will have to use something better!
Because when I had a go at implementing a programming language, I made damn sure array accesses never went out-of-bounds, the only way I knew how: by baking in a % operation. It adds some cycles, for sure; but not as many as redoing everything from scratch because a simple typo caused an unsaved program to overwrite itself.
>Because when I had a go at implementing a programming language, I made damn sure array accesses never went out-of-bounds
Did your programming language support dynamically sized arrays?
One of the problems with K&R C was the behaviour around some library operations wasn't clearly defined. So is a write beyond a boundary always bad, or is it legitimate; just that the developer wasn't practising defensive programming, so implemented it poorly?
The BIG issue with that is you would need to change ALL the calls in the legacy code and then carry out a extremely comprehensive test.
This of course will take a lot of time and money. In a commercial world who will be paying for that! Better to just keep the old code in place doing its job and only, only replace / rewrite when you have to.
Refactoring old code to remove all the dangerous calls to things like memset, memcpy, memcmp, strcpy, strcmp etc will normally end up in a complete rewrite as the old legacy code will have had the boot prints of multiple developers over the years it has been in production. Some of those developers have been good and others well...
Some of the stuff I've worked on has been over 30 years old and has been so bodged and patched that most of the job is just trying to work out what the code does. In some cases there have been bits of code that have been logically commented out because they provided a function that is no longer needed.
This problem is not specific to any language one of the worst legacy systems I was involved with was written in COBOL.
One rule I found useful is "If it is not broken don't fix it!"
The Register 
Biting the hand that feeds IT
Copyright. All rights reserved © 1998–2024
