10 ms·

Every time I see a fiddly fix like this where we're changing a buffer counter to a postfix instead of a prefix increment, I always think "why are we doing this

Every time I see a fiddly fix like this where we're changing a buffer counter to a postfix instead of a prefix increment, I always think "why are we doing this shit ourselves?" We should have standardized a C API to safely read/write buffers from files, networks, etc. decades ago. But here we are in 2020 manually moving pointers around - and what a surprise that we're finding "whoops, another off-by-one error, you know how it goes." No one should be writing new code to read/write bytes. This should be settled. It's like a carpenter building their own nail gun each time they want to build a house.

deleted 7y ago
[deleted]
scintill76 7y ago
> we're changing a buffer counter to a postfix instead of a prefix increment Maybe I'm missing something, but wasn't that a pointless change? The value of the expression was unused, so post- and pre- are equivalent.
- sillysaurusx 7y ago
  Yes. This thread is blind-leading-the-blind. Threads against C almost always are. When I was a pentester, out of more than 50 projects I was on, I always found at least a medium-severity bug (a bug which could allow an attacker to pwn the app using nothing but a web browser). Guess how many of those codebases were written in C? Zero. The smugness is endemic to people outside of security. I was smug too. There are comments I wrote that I cringe at, looking back. Pentesting changes your perspective on things. C isn't going away. Yes, let's make it safer (by pentesting). No, it's not a mortal sin to write C. I'll find ways to exploit your webapp regardless of the language.
  - fulafel 7y ago
    Can I ask what kind of apps you typically worked on? A big part of why people make a big deal of C is that there are lots of code bases that have been under a lot of scrutiny from security minded code reviewers and pentesters and still keep on yielding more vulnerabilities. Ie it's hard to make C safe even when you really try and are a good programmer. In contrast IME most pentesters in the biz work on code bases that have seen much less scrutiny and are often in-house and/or "enterprise" apps whose code was written by people who are probably more domain experts than generally good programmers or security-aware developers...
  - jaas 7y ago
    This is a naive and dangerous perspective. You're basically saying that memory safe languages don't solve all problems so it's fine to use unsafe languages. Don't write code in unsafe languages unless you have to (almost never for new code). Period. You will have some problems either way, but you will have fewer problems in a memory safe language. Pen testing is not even remotely a good mitigation to depend on against the dangers of a language like C. Its fine to do for defense in depth and to find problems not related to memory safety, but it's no substitute for making a more sensible language choice.
    - loeg 7y ago
      Sudo isn't new code in any sense. "Rewrite the world in memory safe languages" is appealing if you completely discount the time and cost required to do so.
      - jaas 7y ago
        I didn't say that.
      - desc 7y ago
        And all the new bugs introduced in the process.
    - pjmlp 7y ago
      That is the classical defense from C camp, going back to the 80's Morris worm.
  - pdimitar 7y ago
    So you are saying pentesting makes C safe? I wouldn't be so sure, I've seen several 50k+ LOC projects in C that had vulnerabilities even after several audits. Is it so wrong to recommend a solution that will kill a good chunk of the problems right at the door? And FYI, you are being pretty smug here as well. The fact that people criticised unrelated parts of the PR in addition to the relevant parts shouldn't give you a free license to dismiss the bigger criticism.
  - cortesoft 7y ago
    The idea that pentesting will make code safe is pretty crazy... This is basically the same as saying C code doesn't have safety issues as long as you code carefully.
  - marcinzm 7y ago
    >I always found at least a medium-severity bug >Guess how many of those codebases were written in C? Zero. So, based on those two comments, you have zero data (anecdotal or other) comparing the rate of bugs in C programs versus other languages since you didn't test any C programs. Code security is not about eliminating bugs but about lowering the rate they show up at since things always get through and money/time is finite. Pen testing doesn't catch everything and many projects that have been tested still yield bugs. There's also a limit to how much money can be spent on testing so using it find (and document, review, etc.) low hanging bugs due to using C means you're more likely to miss a deeper bug. Or in other words, spending the same effort to pen test a safe language will leave fewer bugs left over than spending it to pen test an unsafe language. So using an unsafe language is still worse.
    - asddubs 7y ago
      I'm reminded of Scott Meyers most important design guideline, which I may be paraphrasing slightly, but it goes: "An API should be easy to use correctly and hard to use incorrectly" The dumbest link in the chain is always the guy writing the code, not the programming language, and not the machine executing it. So things should be designed in a way that makes it as hard as possible for programmers to make errors. e: video is here for those who are interested https://www.youtube.com/watch?v=5tg1ONG18H8 https://www.youtube.com/watch?v=5tg1ONG18H8
      - JoshTriplett 7y ago
        That idea is older. Rusty Russell advocated in 2008 that APIs should be "easy to use and hard to misuse". https://ozlabs.org/~rusty/index.cgi/tech/2008-03-30.html https://ozlabs.org/~rusty/index.cgi/tech/2008-03-30.html https://ozlabs.org/~rusty/index.cgi/tech/2008-04-01.html https://ozlabs.org/~rusty/index.cgi/tech/2008-04-01.html
      - tialaramex 7y ago
        Right, this is more broadly applicable than even just APIs or software. Think about how often you blindly inserted a USB A or B (mini-B, micro-B) plug the wrong way first and had to try again. Why is the wrong way even possible? USB C's plug fixes this. A huge improvement to railway safety was the invention of Mechanical Interlocking. It was made essentially impossible to set signals in some inherently unsafe ways. When you tried to give a Clear signal to the Express, through a junction a late running Coal Train is already crossing, the huge metal lever you're trying to move to do that won't budge because all your controls are connected via elaborate mechanical interlocking to prevent this mistake. The signal remains at Danger, the Express stops, tragedy is averted.
  - tialaramex 7y ago
    > When I was a pentester, out of more than 50 projects I was on, I always found at least a medium-severity bug Perhaps you were a good pentester. More likely the code you attacked was rubbish. I worked on a relatively important Internet-facing web application for about a decade, it was pen tested by a variety of outfits both when it was owned by a small start-up and when it was owned by a huge corporation. Good design meant that we never saw any pen tester come anywhere close to "pwn the app" in either its public UI or the APIs which were (by my insistence) publicly accessible rather than wasting our time chasing endless "please change the allowed IP ranges" requests through layers of bureaucracy. My experience was pen testers would bang on the predictable places, try obvious things, maybe trigger some alarms which I then had to pause - and get nowhere. We'd get back a report that said things like if I put in this nonsense email address I get a different error message than this other nonsense email address. OK, that's nice we'll put it on the backlog, did you find any actual security problems? No. I'd say that using a memory safe language (Java) helped make that happen, IMNSHO as a very good C programmer for whom writing it in Java was not my first choice (nor was C).
    - josephg 7y ago
      I think most of the people you'll find writing code in C today have decades of experience programming. And I think the knowledge and experience they have makes a huge difference in practice when it comes to security engineering. In comparison, so much modern javascript is written by junior developers - often without a lot of oversight. Junior developers can't "see" security vulnerabilities yet because they haven't learned to look for them. So I'm not surprised there's lots of critical vulnerabilities in modern software. Many coding bootcamps don't bother to teach any security engineering or best practices, and doing consulting work I've (often by chance) caught a disastrous amount of awful code people have tried to push to production simply because they didn't know any better. A lot of it is really simple stuff - guessable database IDs used as session tokens. JSON objects passed from the browser directly into mongodb. Authentication checks accidentally missing from certain admin dashboard APIs. Passwords and credit card information stored in plaintext, handrolled crypto, and so on. Given the choice between C code and Rust code written by someone who's been programming for 30 years I expect the rust code would be safer. But if I'm asked to choose between C code written by an experienced engineer and javascript code written by a junior engineer, it wouldn't surprise me if it turned out that the C code was on average still more secure.
  - blattimwind 7y ago
    You would expect developers who put security logic bugs in their code to get C's nuances right while implementing the same application in C?
- MaulingMonkey 7y ago
  It is a pointless change in that diff yeah. Perhaps it's meant to match a style guide that recommends preferring postfix operators? Or perhaps it's editing shrapnel from an earlier version of the changelist, that maybe used the result of that statement?
codeflo 7y ago
The prefix/postfix change is a red herring, that doesn't do anything (since the expression result isn't used). Which means that the security fix is interspersed with code style changes. Maybe there's a reason to do this here, but it's often not considered good practice because it makes it harder to review the code. AFAICT, the only real changes are in (new) line 411, resetting the cp variable after the loop, and line 416, ignoring write errors. The former is probably the relevant one.
- MaulingMonkey 7y ago
  198 also disables password feedback mode in !tty mode. So, style changes, behavior changes, and security fixes all in one changelist. This is a small enough diff that I wouldn't complain if given this to review - but I probably also would've reverted the style changes (and would give myself at least 50/50 odds of splitting the changelist in two) Perhaps I've just been too broken by 1000+ LOC diffs that intersperse multiple behavior changes and style and refactoring and ...
- stanferder 7y ago
  > The prefix/postfix change is a red herring, that doesn't do anything The prefix/postfix change does something to the reader. It's a distraction trying to figure out if that change was intended, whether the author knows it has no effect, or whether I'm wrong about the change having no effect. I appreciate the author doing the hard work of fixing this, especially with the code in full view of the public, but if I were an official reviewer I'd ask to get all the unnecessary changes removed.
- nullc 7y ago
  Better than OpenSSL putting out critical security fixes in a 400kloc diff...
- kohtatsu 7y ago
  PSA: git add -p and git commit -p exist and are beautiful.
  - jwilk 7y ago
userbinator 7y ago
If anything the solution is more low-level education, not the exact opposite. People who understand these essential basics, e.g. by starting with Asm, are going to be far less likely to make these sorts of mistakes and write less buggy code in general. I mean, a buffer overflow is a very simple concept and it's not hard to understand. How many bytes could be written, and how much storage is reserved? It's really just arithmetic. The amount of anti-education authoritarian fearmongering in these discussions is disturbing. Then again, given how much everything seems to be rapidly moving in the direction of dystopian corporatocracy, perhaps that's not so surprising. There's already a few comments about how "safer" languages don't really solve anything. They just push the problems up higher in the stack of abstractions... and when that time comes, what are the chances the "security" fearmongerers will just start blaming something else? It's fundamentally a problem of competence, and doing everything you can to make the world a prison in pursuit of that "perfect security" is really not a good idea. There's no replacement for intelligence.
- pjmlp 7y ago
  Security reports from Google and Microsoft's teams prove otherwise. Also let's not forget that C exploits go back to the 80's, when most of us where doing full application in Assembly. Supposedly we knew our pointers pretty well.
  - shakna 7y ago
    > Also let's not forget that C exploits go back to the 80's, when most of us where doing full application in Assembly. I'm not sure this statement adds anything to your argument. "Exploits go back as far as the language was in common use" doesn't really mean anything. I'm sure Ada has had exploits, especially around its early lifetime. But its a language that's supposed to be safe.
    - AgentME 7y ago
      The existence or non-existence of exploits isn't very interesting. The rate that users make vulnerable code is more interesting. The fact that remote-code execution vulnerabilities are still very common in C codebases even after people and the ecosystem have had decades to learn the ins and outs of C is concerning. Maybe the rate of RCE occurrences in C codebases has gone down over time as people have been educated about vulnerabilities more, but I would guess that the rate of RCE vulnerabilities in C codebases is still much higher than the rate ever was for safer languages.
fiddlerwoaroof 7y ago
As far as I can tell, the prefix/postfix thing here is irrelevant and mostly done for style reasons? It’s on its own line and the value of the expression isn’t used, unless I’m missing something obvious.

deleted 7y ago

[deleted]

scintill76 7y ago

> we're changing a buffer counter to a postfix instead of a prefix increment Maybe I'm missing something, but wasn't that a pointless change? The value of the expression was unused, so post- and pre- are equivalent.

sillysaurusx 7y ago
Yes. This thread is blind-leading-the-blind. Threads against C almost always are. When I was a pentester, out of more than 50 projects I was on, I always found at least a medium-severity bug (a bug which could allow an attacker to pwn the app using nothing but a web browser). Guess how many of those codebases were written in C? Zero. The smugness is endemic to people outside of security. I was smug too. There are comments I wrote that I cringe at, looking back. Pentesting changes your perspective on things. C isn't going away. Yes, let's make it safer (by pentesting). No, it's not a mortal sin to write C. I'll find ways to exploit your webapp regardless of the language.
- fulafel 7y ago
  Can I ask what kind of apps you typically worked on? A big part of why people make a big deal of C is that there are lots of code bases that have been under a lot of scrutiny from security minded code reviewers and pentesters and still keep on yielding more vulnerabilities. Ie it's hard to make C safe even when you really try and are a good programmer. In contrast IME most pentesters in the biz work on code bases that have seen much less scrutiny and are often in-house and/or "enterprise" apps whose code was written by people who are probably more domain experts than generally good programmers or security-aware developers...
- jaas 7y ago
  This is a naive and dangerous perspective. You're basically saying that memory safe languages don't solve all problems so it's fine to use unsafe languages. Don't write code in unsafe languages unless you have to (almost never for new code). Period. You will have some problems either way, but you will have fewer problems in a memory safe language. Pen testing is not even remotely a good mitigation to depend on against the dangers of a language like C. Its fine to do for defense in depth and to find problems not related to memory safety, but it's no substitute for making a more sensible language choice.
  - loeg 7y ago
    Sudo isn't new code in any sense. "Rewrite the world in memory safe languages" is appealing if you completely discount the time and cost required to do so.
    - jaas 7y ago
      I didn't say that.
    - desc 7y ago
      And all the new bugs introduced in the process.
  - pjmlp 7y ago
    That is the classical defense from C camp, going back to the 80's Morris worm.
- pdimitar 7y ago
  So you are saying pentesting makes C safe? I wouldn't be so sure, I've seen several 50k+ LOC projects in C that had vulnerabilities even after several audits. Is it so wrong to recommend a solution that will kill a good chunk of the problems right at the door? And FYI, you are being pretty smug here as well. The fact that people criticised unrelated parts of the PR in addition to the relevant parts shouldn't give you a free license to dismiss the bigger criticism.
- cortesoft 7y ago
  The idea that pentesting will make code safe is pretty crazy... This is basically the same as saying C code doesn't have safety issues as long as you code carefully.
- marcinzm 7y ago
  >I always found at least a medium-severity bug >Guess how many of those codebases were written in C? Zero. So, based on those two comments, you have zero data (anecdotal or other) comparing the rate of bugs in C programs versus other languages since you didn't test any C programs. Code security is not about eliminating bugs but about lowering the rate they show up at since things always get through and money/time is finite. Pen testing doesn't catch everything and many projects that have been tested still yield bugs. There's also a limit to how much money can be spent on testing so using it find (and document, review, etc.) low hanging bugs due to using C means you're more likely to miss a deeper bug. Or in other words, spending the same effort to pen test a safe language will leave fewer bugs left over than spending it to pen test an unsafe language. So using an unsafe language is still worse.
  - asddubs 7y ago
    I'm reminded of Scott Meyers most important design guideline, which I may be paraphrasing slightly, but it goes: "An API should be easy to use correctly and hard to use incorrectly" The dumbest link in the chain is always the guy writing the code, not the programming language, and not the machine executing it. So things should be designed in a way that makes it as hard as possible for programmers to make errors. e: video is here for those who are interested https://www.youtube.com/watch?v=5tg1ONG18H8 https://www.youtube.com/watch?v=5tg1ONG18H8
    - JoshTriplett 7y ago
      That idea is older. Rusty Russell advocated in 2008 that APIs should be "easy to use and hard to misuse". https://ozlabs.org/~rusty/index.cgi/tech/2008-03-30.html https://ozlabs.org/~rusty/index.cgi/tech/2008-03-30.html https://ozlabs.org/~rusty/index.cgi/tech/2008-04-01.html https://ozlabs.org/~rusty/index.cgi/tech/2008-04-01.html
    - tialaramex 7y ago
      Right, this is more broadly applicable than even just APIs or software. Think about how often you blindly inserted a USB A or B (mini-B, micro-B) plug the wrong way first and had to try again. Why is the wrong way even possible? USB C's plug fixes this. A huge improvement to railway safety was the invention of Mechanical Interlocking. It was made essentially impossible to set signals in some inherently unsafe ways. When you tried to give a Clear signal to the Express, through a junction a late running Coal Train is already crossing, the huge metal lever you're trying to move to do that won't budge because all your controls are connected via elaborate mechanical interlocking to prevent this mistake. The signal remains at Danger, the Express stops, tragedy is averted.
- tialaramex 7y ago
  > When I was a pentester, out of more than 50 projects I was on, I always found at least a medium-severity bug Perhaps you were a good pentester. More likely the code you attacked was rubbish. I worked on a relatively important Internet-facing web application for about a decade, it was pen tested by a variety of outfits both when it was owned by a small start-up and when it was owned by a huge corporation. Good design meant that we never saw any pen tester come anywhere close to "pwn the app" in either its public UI or the APIs which were (by my insistence) publicly accessible rather than wasting our time chasing endless "please change the allowed IP ranges" requests through layers of bureaucracy. My experience was pen testers would bang on the predictable places, try obvious things, maybe trigger some alarms which I then had to pause - and get nowhere. We'd get back a report that said things like if I put in this nonsense email address I get a different error message than this other nonsense email address. OK, that's nice we'll put it on the backlog, did you find any actual security problems? No. I'd say that using a memory safe language (Java) helped make that happen, IMNSHO as a very good C programmer for whom writing it in Java was not my first choice (nor was C).
  - josephg 7y ago
    I think most of the people you'll find writing code in C today have decades of experience programming. And I think the knowledge and experience they have makes a huge difference in practice when it comes to security engineering. In comparison, so much modern javascript is written by junior developers - often without a lot of oversight. Junior developers can't "see" security vulnerabilities yet because they haven't learned to look for them. So I'm not surprised there's lots of critical vulnerabilities in modern software. Many coding bootcamps don't bother to teach any security engineering or best practices, and doing consulting work I've (often by chance) caught a disastrous amount of awful code people have tried to push to production simply because they didn't know any better. A lot of it is really simple stuff - guessable database IDs used as session tokens. JSON objects passed from the browser directly into mongodb. Authentication checks accidentally missing from certain admin dashboard APIs. Passwords and credit card information stored in plaintext, handrolled crypto, and so on. Given the choice between C code and Rust code written by someone who's been programming for 30 years I expect the rust code would be safer. But if I'm asked to choose between C code written by an experienced engineer and javascript code written by a junior engineer, it wouldn't surprise me if it turned out that the C code was on average still more secure.
- blattimwind 7y ago
  You would expect developers who put security logic bugs in their code to get C's nuances right while implementing the same application in C?
MaulingMonkey 7y ago
It is a pointless change in that diff yeah. Perhaps it's meant to match a style guide that recommends preferring postfix operators? Or perhaps it's editing shrapnel from an earlier version of the changelist, that maybe used the result of that statement?

codeflo 7y ago

The prefix/postfix change is a red herring, that doesn't do anything (since the expression result isn't used). Which means that the security fix is interspersed with code style changes. Maybe there's a reason to do this here, but it's often not considered good practice because it makes it harder to review the code. AFAICT, the only real changes are in (new) line 411, resetting the cp variable after the loop, and line 416, ignoring write errors. The former is probably the relevant one.

MaulingMonkey 7y ago
198 also disables password feedback mode in !tty mode. So, style changes, behavior changes, and security fixes all in one changelist. This is a small enough diff that I wouldn't complain if given this to review - but I probably also would've reverted the style changes (and would give myself at least 50/50 odds of splitting the changelist in two) Perhaps I've just been too broken by 1000+ LOC diffs that intersperse multiple behavior changes and style and refactoring and ...
stanferder 7y ago
> The prefix/postfix change is a red herring, that doesn't do anything The prefix/postfix change does something to the reader. It's a distraction trying to figure out if that change was intended, whether the author knows it has no effect, or whether I'm wrong about the change having no effect. I appreciate the author doing the hard work of fixing this, especially with the code in full view of the public, but if I were an official reviewer I'd ask to get all the unnecessary changes removed.
nullc 7y ago
Better than OpenSSL putting out critical security fixes in a 400kloc diff...
kohtatsu 7y ago
PSA: git add -p and git commit -p exist and are beautiful.
- jwilk 7y ago

userbinator 7y ago

If anything the solution is more low-level education, not the exact opposite. People who understand these essential basics, e.g. by starting with Asm, are going to be far less likely to make these sorts of mistakes and write less buggy code in general. I mean, a buffer overflow is a very simple concept and it's not hard to understand. How many bytes could be written, and how much storage is reserved? It's really just arithmetic. The amount of anti-education authoritarian fearmongering in these discussions is disturbing. Then again, given how much everything seems to be rapidly moving in the direction of dystopian corporatocracy, perhaps that's not so surprising. There's already a few comments about how "safer" languages don't really solve anything. They just push the problems up higher in the stack of abstractions... and when that time comes, what are the chances the "security" fearmongerers will just start blaming something else? It's fundamentally a problem of competence, and doing everything you can to make the world a prison in pursuit of that "perfect security" is really not a good idea. There's no replacement for intelligence.

pjmlp 7y ago
Security reports from Google and Microsoft's teams prove otherwise. Also let's not forget that C exploits go back to the 80's, when most of us where doing full application in Assembly. Supposedly we knew our pointers pretty well.
- shakna 7y ago
  > Also let's not forget that C exploits go back to the 80's, when most of us where doing full application in Assembly. I'm not sure this statement adds anything to your argument. "Exploits go back as far as the language was in common use" doesn't really mean anything. I'm sure Ada has had exploits, especially around its early lifetime. But its a language that's supposed to be safe.
  - AgentME 7y ago
    The existence or non-existence of exploits isn't very interesting. The rate that users make vulnerable code is more interesting. The fact that remote-code execution vulnerabilities are still very common in C codebases even after people and the ecosystem have had decades to learn the ins and outs of C is concerning. Maybe the rate of RCE occurrences in C codebases has gone down over time as people have been educated about vulnerabilities more, but I would guess that the rate of RCE vulnerabilities in C codebases is still much higher than the rate ever was for safer languages.

fiddlerwoaroof 7y ago

As far as I can tell, the prefix/postfix thing here is irrelevant and mostly done for style reasons? It’s on its own line and the value of the expression isn’t used, unless I’m missing something obvious.