Hi,
The Bug#430387 might be a security risk.
When --use-unzip option are used, fcrackzip passes the filename of zipfile to
shell without escaping.
And if the name of zipfile contains ";Malicious Code;", that Malicious Code will
be executed by shell.
Here is a example:
1) Create a zipfile that contains ";Malicious Code;". In this example, I have
used 'touch TEST' as a Malicious Code.
$ cp /usr/share/doc/fcrackzip/examples/noradi.zip "noradi;touch TEST;.zip"
$ ls
noradi;touch TEST;.zip
2) Execute fcrackzip with --use-unzip option.
$ fcrackzip -b -c a -p aaaaaa --use-unzip "noradi;touch TEST;.zip"
unzip: cannot find or open noradi, noradi.zip or noradi.ZIP.
<Press Ctrl-C to stop>
3) A file TEST is created.
$ ls
TEST noradi;touch TEST;.zip
Note: ';' character is a separator of the multiple commands.
Thus the file "noradi;touch TEST;.zip" is interpreted as following three
commands.
$ unzip -qqtP pw noradi
$ touch TEST
$ .zip
The problem is that fcrackzip uses system(3) to execute unzip.
The arguments of system(3) must be safe since system(3) uses 'sh -c' to execute
a external command.
It is better to use fork+exec rather than system(3), because fork+exec can
bypass the shell to be called.
(fork+exec has additional advantage.
Because system(3) ignores some signals include SIGINT, fcrackzip sometimes
cannot be interrupted by Ctrl-C.
fork+exec has no such problem.)
I have made a patch to fix this problem.
This patch completely rewrites check_unzip function to use fork+exec.
Regards,
Morita Sho
--- fcrackzip-0.3.orig/main.c 2007-10-05 23:12:25.000000000 +0900
+++ fcrackzip-0.3/main.c 2007-10-06 00:17:15.000000000 +0900
@@ -27,11 +27,13 @@
#include <string.h>
#ifdef USE_UNIX_REDIRECTION
-#define DEVNULL ">/dev/null 2>&1"
+#define DEVNULL "/dev/null"
#else
-#define DEVNULL ">NUL 2>&1"
+#define DEVNULL "NUL"
#endif
+#include <errno.h>
+
#include "crack.h"
int use_unzip;
@@ -47,38 +49,76 @@
int REGPARAM
check_unzip (const char *pw)
{
- char buff[1024];
+ pid_t cpid;
+ cpid = fork ();
+ if (cpid == -1)
+ {
+ perror ("fork");
+ exit (EXIT_FAILURE);
+ }
+
+ if (cpid == 0)
+ {
+ // Redirect STDERR/STDOUT to /dev/null
+ int oldfd_stderr, oldfd_stdout;
+ oldfd_stdout = dup (fileno (stdout));
+ if (oldfd_stdout == -1)
+ {
+ perror ("dup for stdout");
+ _exit (127);
+ }
+ oldfd_stderr = dup (fileno (stderr));
+ if (oldfd_stderr == -1)
+ {
+ perror ("dup for stderr");
+ _exit (127);
+ }
+ if (freopen (DEVNULL, "w", stdout) == NULL)
+ {
+ perror ("freopen " DEVNULL " for stdout");
+ _exit (127);
+ }
+ if (freopen (DEVNULL, "w", stderr) == NULL)
+ {
+ perror ("freopen " DEVNULL " for stderr");
+ _exit (127);
+ }
+ execlp ("unzip", "unzip", "-qqtP", pw, file_path[0], NULL);
+
+ // When execlp failed.
+ // Restores the stderr/stdout redirection to print an error.
+ int errno_saved = errno;
+ dup2 (oldfd_stderr, fileno (stderr));
+ dup2 (oldfd_stdout, fileno (stdout));
+ close (oldfd_stderr);
+ close (oldfd_stdout);
+ errno = errno_saved;
+ perror ("execlp for unzip");
+ _exit (127); // Returns 127 on error as system(3) does
+ }
+
int status;
-
- /* shell special char patch, to be changed with something quicker */
- char patched_pw[1004];
-
- int count = strlen(pw), i, j = 0;
- for (i = 0; i < count; i++){
- if ((pw[i] == '!') || (pw[i] == '$') || (pw[i] == '&') || (pw[i] == '(') || (pw[i] == ')') || (pw[i] == '#') || (pw[i] == ' ')) {
- patched_pw[j] = '\\';
- j++;
- }
- patched_pw[j] = pw[i];
- j++;
- }
- patched_pw[j] = '\0';
-
- sprintf (buff, "unzip -qqtP %s %s " DEVNULL, patched_pw, file_path[0]);
-
- status = system (buff);
-
-#undef REDIR
-/* In case of "stored" items, unzip returns 1.
- * In case of wrong password, the returned value is 122.
- */
- if ((status == EXIT_SUCCESS) || (WEXITSTATUS(status) == 1))
+ if (waitpid (cpid, &status, 0) == -1)
{
- printf("\n\nPASSWORD FOUND!!!!: pw == %s\n", pw);
+ perror ("waitpid");
+ exit (EXIT_FAILURE);
+ }
+
+ // The child process does not terminated normally, OR returns the exit status 127.
+ if (!WIFEXITED (status)
+ || (WIFEXITED (status) && (WEXITSTATUS (status) == 127)))
+ {
+ fprintf (stderr, "Executing unzip failed.\n");
+ exit (EXIT_FAILURE);
+ }
+ // unzip exited normally with the exit status 0 then...
+ if (WIFEXITED (status) && (WEXITSTATUS (status) == EXIT_SUCCESS))
+ {
+ printf ("\n\nPASSWORD FOUND!!!!: pw == %s\n", pw);
exit (EXIT_SUCCESS);
}
- return !status;
+ return 0;
}
/* misc. callbacks. */
