2016-02-15 23:39:27 UTC
I think the ExecOnCrackedPassword feature, which just got in, is
unacceptable as currently implemented. I understand that jumbo is crap
and anything goes (and core is only moderately better), but maybe this
went too far. OK, I start to sound like Linus here. Let me calm down
after seeing this. ;-)
We could add a huge warning about just how very insecure this feature
is (in multiple ways, in fact), but even then it's also unreliable,
since it exec's the program via system(), so it would fail on shell
escapes seen in passwords.
Maybe we should revert those commits for now, and use this opportunity
to set some minimum pre-commit quality standards for jumbo?
As to the feature, I understand why it may be desirable, so maybe it can
be reimplemented with passing of the two strings (username and password)
via stdin (the example bash script would then use "read"). Even then,
there would need to be a separator character, which could occur in a
username... but luckily (for this) we don't currently support ':' (by
default), linefeed, and NUL in usernames. So maybe just use linefeed.