Just in from “C is fine just know what you’re doing”.

> Exploiting the bug does not require sudo permissions, merely that pwfeedback be enabled.

> the stack overflow may allow unprivileged users to escalate to the root account. Because the attacker has complete control of the data used to overflow the buffer, there is a high likelihood of exploitability.

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.

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.

PSA: git add -p and git commit -p exist and are beautiful.

FWIW, sudo uses Mercurial, not git.

https://github.com/sudo-project/sudo is just a mirror.