From: [email protected]

Package: john
Version: 1.6.36-4
Severity: normal
Tags: experimental patch

Hello,

I'm experiencing some segmentation faults with the --restore and
--status options. Here is how it can be reproduced:
$ /usr/sbin/john testcase.2
After a few seconds I stop the session (Ctrl-C)
the john.rec was created in my ~/.john directory
I'm using john with the following testcase (any non-trivial password
should be OK):
root:RfiUAvoW5xYKk:0:0:root:/root:/bin/bash
(testcase.2 with a one character diff)

Then I wanted to restore it:
$ /usr/sbin/john --restore
Segmentation fault

Asking for the status leads to another segfaut:
$ /usr/sbin/john --status
Segmentation fault

I'm giving lots of details of my bug analysis, because I'm not sure of
the attached patch.



The segmentation fault occurs in path_expand which is called with
a NULL filename.
Fixing path_expand with "if (!name) return NULL;" won't help, because
the problem is that there is no session name.

In the upstream source, there is a default session name (RECOVERY_NAME),
but the --private patch initialized rec_name to NULL, and set it later to private_path(RECOVERY_NAME) in rec_init.
In the case of the --status and --restore options, rec_restore_args is
called before rec_init. So I added:
if ( rec_name == NULL )
rec_name = private_path(RECOVERY_NAME);
at the beginning of rec_restore_args (note: it is still needed for rec_init)

But then, it fails in path_expand again, because path_init wasn't called
before option_init, and user_home_path is empty.
The call order of opt_init and path_init was changed in Debian to add
the --private option (which made path_init dependant of options).

The patch I propose is to split path_init in two parts, the second part
will be in charge to create the private directory and will be called
after opt_init (and I hope the private directory is not needed for
status_init and opt_init).

With the attached patch, I noticed the following behavior:
"john --status" will use the ~/.john/john.rec file, even if the user is
in the directory of a previous session. He will then have to use either
"john --private=. --status" or "john --status=john"
I don't know if it is the expected behavior.

By the way, the patch fix some other issues:
- in private_path, size was too short by one byte => the string can be
non-'\0' terminated which leads to segfaults (this was needed for a
proper operation of the patch)
- add the OPT_REQ_PARAM flag to --config and --private (this should
fix the segmentation fault mentioned in the TODO file).
- remove the FLG_PRIVATE flag to allow using --private with other
options. I test options.private to know if the --private option was
set.
- remove some compilation warning:
+ change prototypes of dummy_rules_apply, and the apply types in
wordlist.c
+ replace malloc by mem_alloc in private_path
+ include <unistd.h> for _exit in signals.c
(Note: new upstream version also fix the strict aliasing warnings)


Note:
There is a very simple solution to avoid those segmentation faults with
the current package:
using a named session (with --session=foo and --restore=foo, for
example, --status=john in the ~/.john directory will show the status of
my previous session).


[Update]:
I wanted to know how the cronjob could restore its session, and have
seen that it was possible to provide the path of the session. I tried:
$ john --status=~/.john/john
fopen: ~/.john/john: No such file or directory
This unfortunately doesn't work because of the dot
$ john --status=~/.john/john.rec
will work. A second patch is attached which should fix this.
This one could probably be forwarded upstream.

I still think the first patch should be applied.


-- System Information:
Debian Release: 3.1
APT prefers unstable
APT policy: (500, 'unstable'), (1, 'experimental')
Architecture: i386 (i686)
Kernel: Linux 2.4.26
Locale: LANG=fr_FR.UTF-8, LC_CTYPE=fr_FR.UTF-8

Versions of packages john depends on:
ii debconf 1.4.30 Debian configuration management sy ii libc6 2.3.2.ds1-15 GNU C Library: Shared libraries an

-- debconf information:
john/noconfigfile:
john/wordlist: /usr/share/john/password.lst
* john/cronjob: true
* john/cronjob-replacement: true
john/no-replacement:

Kind Regards,
--
Nekral

diff -raup ../orig/src/john.c src/john.c
--- ../orig/src/john.c 2004-08-12 22:51:54.000000000 +0200
+++ src/john.c 2004-08-13 15:48:14.000000000 +0200
@@ -231,9 +231,10 @@ static void john_init(int argc, char **a
#endif


+ path_init(argv);
status_init(NULL, 1);
opt_init(argc, argv);
- path_init(argv);
+ create_private_home();
if (options.flags & FLG_CFGFILE)
/* If the user has defined a config file, use this one or exit */
cfg_init(options.config,0);
diff -raup ../orig/src/options.c src/options.c
--- ../orig/src/options.c 2004-08-12 22:51:54.000000000 +0200
+++ src/options.c 2004-08-13 15:48:14.000000000 +0200
@@ -67,10 +67,10 @@ static struct opt_entry opt_list[] = {
"%u", &mem_saving_level},
/* Added to have personalised locations of files */
{"config", FLG_CFGFILE, FLG_CRACKING_CHK,
- 0,0, OPT_FMT_STR_ALLOC, &options.config},
+ 0, OPT_REQ_PARAM, OPT_FMT_STR_ALLOC, &options.config},
#ifdef JOHN_PRIVATE_HOME
- {"private", FLG_PRIVATE, FLG_CRACKING_CHK,
- 0,0, OPT_FMT_STR_ALLOC, &options.private},
+