[john-dev] ExecOnCrackedPassword

[Solar Designer]

magnum -

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.

Alexander

[magnum]

Post by Solar Designer
I think the ExecOnCrackedPassword feature, which just got in, is
unacceptable as currently implemented. (...)

I wasn't expecting you to love it ;-)

Post by Solar Designer
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?

Right, I was already leaning towards that conclusion while handling
https://github.com/magnumripper/JohnTheRipper/pull/2055

Post by Solar Designer
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.

Maybe. For now I'll revert the commits and open an issue for the feature
instead. I wont accept PR's until we have agreed on the details.

magnum