Buffer overflow when pwfeedback is set in sudoers

Re:Buffer overflow when pwfeedback is set in sudoers by codeflo on Sunday February 02, 2020 @01:50PM (#22218563)

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.

[ Parent | on HN ]

Re:Buffer overflow when pwfeedback is set in sudoers by MaulingMonkey on Sunday February 02, 2020 @02:00PM (#22218619)

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 ...

[ Parent | on HN ]

Re:Buffer overflow when pwfeedback is set in sudoers by stanferder on Sunday February 02, 2020 @03:24PM (#22219203)

> 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.

[ Parent | on HN ]

Re:Buffer overflow when pwfeedback is set in sudoers by nullc on Sunday February 02, 2020 @04:18PM (#22219504)

Better than OpenSSL putting out critical security fixes in a 400kloc diff...

[ Parent | on HN ]

Re:Buffer overflow when pwfeedback is set in sudoers by kohtatsu on Sunday February 02, 2020 @11:35PM (#22221441)

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

[ Parent | on HN ]

Re:Buffer overflow when pwfeedback is set in sudoers by jwilk on Monday February 03, 2020 @10:31AM (#22224975)

FWIW, sudo uses Mercurial, not git.

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

[ Parent | on HN ]

Re:Buffer overflow when pwfeedback is set in sudoers by masklinn on Monday February 03, 2020 @01:22PM (#22227198)

Doesn't really make a difference, hg offers similar facilities (formerly the record extension, now hg commit -i). There's no index, but it's essentially the same as git commit -p.

[ Parent | on HN ]