This is a live mirror of the Perl 5 development currently hosted at https://github.com/perl/perl5
[perl #126922] avoid access to uninitialized memory in win32 crypt()
authorTony Cook <tony@develop-help.com>
Thu, 17 Dec 2015 00:15:31 +0000 (11:15 +1100)
committerTony Cook <tony@develop-help.com>
Mon, 4 Jan 2016 23:38:38 +0000 (10:38 +1100)
Previously the Win32 crypt implementation() would access the first
and second characters of the salt, even if the salt was zero length.

Add validation that will detect both a short salt and invalid
characters in the salt.

MANIFEST
t/win32/crypt.t [new file with mode: 0644]
win32/fcrypt.c

index b2c0264..8844511 100644 (file)
--- a/MANIFEST
+++ b/MANIFEST
@@ -5593,6 +5593,7 @@ t/uni/universal.t         See if Unicode in calls to UNIVERSAL works
 t/uni/upper.t                  See if Unicode casing works
 t/uni/variables.t              See that the rules for variable names work
 t/uni/write.t                  See if Unicode formats work
+t/win32/crypt.t                        Test Win32 crypt for compatibility
 t/win32/fs.t                   Test Win32 link for compatibility
 t/win32/popen.t                        Test for stdout races in backticks, etc
 t/win32/runenv.t               Test if Win* perl honors its env variables
diff --git a/t/win32/crypt.t b/t/win32/crypt.t
new file mode 100644 (file)
index 0000000..f0e89ab
--- /dev/null
@@ -0,0 +1,41 @@
+#!./perl
+
+BEGIN {
+    chdir 't' if -d 't';
+    @INC = '../lib';
+    require "./test.pl";
+    eval 'use Errno';
+    die $@ if $@ and !is_miniperl();
+}
+
+my @bad_salts =
+   (
+    [ '',   'zero-length' ],
+    [ 'a',  'length 1' ],
+    [ '!a', 'bad first character' ],
+    [ 'a!', 'bad second character' ],
+    [ '@a', 'fencepost before A' ],
+    [ '[a', 'fencepost after Z' ],
+    [ '`a', 'fencepost before a' ],
+    [ '{a', 'fencepost after z' ],
+    [ '-a', 'fencepost before .' ],
+    [ ':a', 'fencepost after 9' ],
+   );
+
+my @good_salts = qw(aa zz AA ZZ .. 99);
+
+plan tests => 2 * @bad_salts + 1 + @good_salts;
+
+for my $bad_salt (@bad_salts) {
+    my ($salt, $what) = @$bad_salt;
+    $! = 0;
+    is(crypt("abc", $salt), undef, "bad salt ($what)");
+    is(0+$!, &Errno::EINVAL, "check errno ($what)");
+}
+
+is(crypt("abcdef", "ab"), "abDMWw5NL.afs", "sanity check result");
+
+# just to check we're not rejecting any good salts
+for my $good_salt (@good_salts) {
+    isnt(crypt("abcdef", $good_salt), undef, "good salt $good_salt");
+}
index fd42d75..4433e68 100644 (file)
@@ -1,6 +1,7 @@
 /* fcrypt.c */
 /* Copyright (C) 1993 Eric Young - see README for more details */
 #include <stdio.h>
+#include <errno.h>
 
 /* Eric Young.
  * This version of crypt has been developed from my MIT compatable
@@ -464,6 +465,14 @@ unsigned const char cov_2char[64]={
 0x73,0x74,0x75,0x76,0x77,0x78,0x79,0x7A
 };
 
+/* the salt for classic DES crypt (which is all we implement here)
+   permits [./0-9A-Za-z], since '.' and '/' immediately preceed
+   '0' we don't need individual checks for '.' and '/' 
+*/
+#define good_for_salt(c) \
+    ((c) >= '.' && (c) <= '9' || (c) >= 'A' && (c) <= 'Z' ||  \
+     (c) >= 'a' && (c) <= 'z')
+
 char *
 des_fcrypt(const char *buf, const char *salt, char *buff)
        {
@@ -476,6 +485,11 @@ des_fcrypt(const char *buf, const char *salt, char *buff)
        unsigned char *b=bb;
        unsigned char c,u;
 
+        if (!good_for_salt(salt[0]) || !good_for_salt(salt[1])) {
+            errno = EINVAL;
+            return NULL;
+        }
+
        /* eay 25/08/92
         * If you call crypt("pwd","*") as often happens when you
         * have * as the pwd field in /etc/passwd, the function