From 56232f9879768e961485d8ba218da18c38768413 Mon Sep 17 00:00:00 2001
From: Noah Misch <noah@leadboat.com>
Date: Mon, 5 Oct 2015 10:06:29 -0400
Subject: [PATCH] pgcrypto: Detect and report too-short crypt() salts.
Certain short salts crashed the backend or disclosed a few bytes of
backend memory. For existing salt-induced error conditions, emit a
message saying as much. Back-patch to 9.0 (all supported versions).
Josh Kupershmidt
Security: CVE-2015-5288
---
contrib/pgcrypto/crypt-blowfish.c | 19 ++++++++++++++--
contrib/pgcrypto/crypt-des.c | 22 +++++++++++++++---
contrib/pgcrypto/expected/crypt-blowfish.out | 9 ++++++++
contrib/pgcrypto/expected/crypt-des.out | 4 ++++
contrib/pgcrypto/expected/crypt-xdes.out | 24 ++++++++++++++++++++
contrib/pgcrypto/px-crypt.c | 2 +-
contrib/pgcrypto/sql/crypt-blowfish.sql | 9 ++++++++
contrib/pgcrypto/sql/crypt-des.sql | 4 ++++
contrib/pgcrypto/sql/crypt-xdes.sql | 16 +++++++++++++
9 files changed, 103 insertions(+), 6 deletions(-)
diff --git a/contrib/pgcrypto/crypt-blowfish.c b/contrib/pgcrypto/crypt-blowfish.c
index fbaa3d776a0..4054e6a06fc 100644
--- a/contrib/pgcrypto/crypt-blowfish.c
+++ b/contrib/pgcrypto/crypt-blowfish.c
@@ -602,6 +602,17 @@ _crypt_blowfish_rn(const char *key, const char *setting,
if (size < 7 + 22 + 31 + 1)
return NULL;
+ /*
+ * Blowfish salt value must be formatted as follows: "$2a$" or "$2x$", a
+ * two digit cost parameter, "$", and 22 digits from the alphabet
+ * "./0-9A-Za-z". -- from the PHP crypt docs. Apparently we enforce a few
+ * more restrictions on the count in the salt as well.
+ */
+ if (strlen(setting) < 29)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("invalid salt")));
+
if (setting[0] != '$' ||
setting[1] != '2' ||
(setting[2] != 'a' && setting[2] != 'x') ||
@@ -611,14 +622,18 @@ _crypt_blowfish_rn(const char *key, const char *setting,
(setting[4] == '3' && setting[5] > '1') ||
setting[6] != '$')
{
- return NULL;
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("invalid salt")));
}
count = (BF_word) 1 << ((setting[4] - '0') * 10 + (setting[5] - '0'));
if (count < 16 || BF_decode(data.binary.salt, &setting[7], 16))
{
px_memset(data.binary.salt, 0, sizeof(data.binary.salt));
- return NULL;
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("invalid salt")));
}
BF_swap(data.binary.salt, 4);
diff --git a/contrib/pgcrypto/crypt-des.c b/contrib/pgcrypto/crypt-des.c
index 4ed44beeff5..2108be8c379 100644
--- a/contrib/pgcrypto/crypt-des.c
+++ b/contrib/pgcrypto/crypt-des.c
@@ -681,9 +681,19 @@ px_crypt_des(const char *key, const char *setting)
if (*setting == _PASSWORD_EFMT1)
{
/*
- * "new"-style: setting - underscore, 4 bytes of count, 4 bytes of
- * salt key - unlimited characters
+ * "new"-style: setting must be a 9-character (underscore, then 4
+ * bytes of count, then 4 bytes of salt) string. See CRYPT(3) under
+ * the "Extended crypt" heading for further details.
+ *
+ * Unlimited characters of the input key are used. This is known as
+ * the "Extended crypt" DES method.
+ *
*/
+ if (strlen(setting) < 9)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("invalid salt")));
+
for (i = 1, count = 0L; i < 5; i++)
count |= ascii_to_bin(setting[i]) << (i - 1) * 6;
@@ -723,10 +733,16 @@ px_crypt_des(const char *key, const char *setting)
#endif /* !DISABLE_XDES */
{
/*
- * "old"-style: setting - 2 bytes of salt key - up to 8 characters
+ * "old"-style: setting - 2 bytes of salt key - only up to the first 8
+ * characters of the input key are used.
*/
count = 25;
+ if (strlen(setting) < 2)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("invalid salt")));
+
salt = (ascii_to_bin(setting[1]) << 6)
| ascii_to_bin(setting[0]);
diff --git a/contrib/pgcrypto/expected/crypt-blowfish.out b/contrib/pgcrypto/expected/crypt-blowfish.out
index 329d78f6254..d79b0c047b4 100644
--- a/contrib/pgcrypto/expected/crypt-blowfish.out
+++ b/contrib/pgcrypto/expected/crypt-blowfish.out
@@ -13,6 +13,15 @@ SELECT crypt('foox', '$2a$06$RQiOJ.3ELirrXwxIZY8q0O');
$2a$06$RQiOJ.3ELirrXwxIZY8q0OR3CVJrAfda1z26CCHPnB6mmVZD8p0/C
(1 row)
+-- error, salt too short:
+SELECT crypt('foox', '$2a$');
+ERROR: invalid salt
+-- error, first digit of count in salt invalid
+SELECT crypt('foox', '$2a$40$RQiOJ.3ELirrXwxIZY8q0O');
+ERROR: invalid salt
+-- error, count in salt too small
+SELECT crypt('foox', '$2a$00$RQiOJ.3ELirrXwxIZY8q0O');
+ERROR: invalid salt
CREATE TABLE ctest (data text, res text, salt text);
INSERT INTO ctest VALUES ('password', '', '');
UPDATE ctest SET salt = gen_salt('bf', 8);
diff --git a/contrib/pgcrypto/expected/crypt-des.out b/contrib/pgcrypto/expected/crypt-des.out
index b8b605037d4..a462dcd580a 100644
--- a/contrib/pgcrypto/expected/crypt-des.out
+++ b/contrib/pgcrypto/expected/crypt-des.out
@@ -13,6 +13,10 @@ SELECT crypt('foox', 'NB');
NB53EGGqrrb5E
(1 row)
+-- We are supposed to pass in a 2-character salt.
+-- error since salt is too short:
+SELECT crypt('password', 'a');
+ERROR: invalid salt
CREATE TABLE ctest (data text, res text, salt text);
INSERT INTO ctest VALUES ('password', '', '');
UPDATE ctest SET salt = gen_salt('des');
diff --git a/contrib/pgcrypto/expected/crypt-xdes.out b/contrib/pgcrypto/expected/crypt-xdes.out
index cdcdefb1996..8cf907512f6 100644
--- a/contrib/pgcrypto/expected/crypt-xdes.out
+++ b/contrib/pgcrypto/expected/crypt-xdes.out
@@ -13,6 +13,30 @@ SELECT crypt('foox', '_J9..j2zz');
_J9..j2zzAYKMvO2BYRY
(1 row)
+-- check XDES handling of keys longer than 8 chars
+SELECT crypt('longlongpassword', '_J9..j2zz');
+ crypt
+----------------------
+ _J9..j2zz4BeseiQNwUg
+(1 row)
+
+-- error, salt too short
+SELECT crypt('foox', '_J9..BWH');
+ERROR: invalid salt
+-- error, count specified in the second argument is 0
+SELECT crypt('password', '_........');
+ERROR: crypt(3) returned NULL
+-- error, count will wind up still being 0 due to invalid encoding
+-- of the count: only chars ``./0-9A-Za-z' are valid
+SELECT crypt('password', '_..!!!!!!');
+ERROR: crypt(3) returned NULL
+-- count should be non-zero here, will work
+SELECT crypt('password', '_/!!!!!!!');
+ crypt
+----------------------
+ _/!!!!!!!zqM49hRzxko
+(1 row)
+
CREATE TABLE ctest (data text, res text, salt text);
INSERT INTO ctest VALUES ('password', '', '');
UPDATE ctest SET salt = gen_salt('xdes', 1001);
diff --git a/contrib/pgcrypto/px-crypt.c b/contrib/pgcrypto/px-crypt.c
index 7b003a76ca6..e3246fc5b9d 100644
--- a/contrib/pgcrypto/px-crypt.c
+++ b/contrib/pgcrypto/px-crypt.c
@@ -42,7 +42,7 @@ run_crypt_des(const char *psw, const char *salt,
char *res;
res = px_crypt_des(psw, salt);
- if (strlen(res) > len - 1)
+ if (res == NULL || strlen(res) > len - 1)
return NULL;
strcpy(buf, res);
return buf;
diff --git a/contrib/pgcrypto/sql/crypt-blowfish.sql b/contrib/pgcrypto/sql/crypt-blowfish.sql
index 60c11400555..3b5a681c3f5 100644
--- a/contrib/pgcrypto/sql/crypt-blowfish.sql
+++ b/contrib/pgcrypto/sql/crypt-blowfish.sql
@@ -6,6 +6,15 @@ SELECT crypt('', '$2a$06$RQiOJ.3ELirrXwxIZY8q0O');
SELECT crypt('foox', '$2a$06$RQiOJ.3ELirrXwxIZY8q0O');
+-- error, salt too short:
+SELECT crypt('foox', '$2a$');
+
+-- error, first digit of count in salt invalid
+SELECT crypt('foox', '$2a$40$RQiOJ.3ELirrXwxIZY8q0O');
+
+-- error, count in salt too small
+SELECT crypt('foox', '$2a$00$RQiOJ.3ELirrXwxIZY8q0O');
+
CREATE TABLE ctest (data text, res text, salt text);
INSERT INTO ctest VALUES ('password', '', '');
diff --git a/contrib/pgcrypto/sql/crypt-des.sql b/contrib/pgcrypto/sql/crypt-des.sql
index fabdc652fc9..a85ec1e6555 100644
--- a/contrib/pgcrypto/sql/crypt-des.sql
+++ b/contrib/pgcrypto/sql/crypt-des.sql
@@ -6,6 +6,10 @@ SELECT crypt('', 'NB');
SELECT crypt('foox', 'NB');
+-- We are supposed to pass in a 2-character salt.
+-- error since salt is too short:
+SELECT crypt('password', 'a');
+
CREATE TABLE ctest (data text, res text, salt text);
INSERT INTO ctest VALUES ('password', '', '');
diff --git a/contrib/pgcrypto/sql/crypt-xdes.sql b/contrib/pgcrypto/sql/crypt-xdes.sql
index d4a74f76bde..8171cd872be 100644
--- a/contrib/pgcrypto/sql/crypt-xdes.sql
+++ b/contrib/pgcrypto/sql/crypt-xdes.sql
@@ -6,6 +6,22 @@ SELECT crypt('', '_J9..j2zz');
SELECT crypt('foox', '_J9..j2zz');
+-- check XDES handling of keys longer than 8 chars
+SELECT crypt('longlongpassword', '_J9..j2zz');
+
+-- error, salt too short
+SELECT crypt('foox', '_J9..BWH');
+
+-- error, count specified in the second argument is 0
+SELECT crypt('password', '_........');
+
+-- error, count will wind up still being 0 due to invalid encoding
+-- of the count: only chars ``./0-9A-Za-z' are valid
+SELECT crypt('password', '_..!!!!!!');
+
+-- count should be non-zero here, will work
+SELECT crypt('password', '_/!!!!!!!');
+
CREATE TABLE ctest (data text, res text, salt text);
INSERT INTO ctest VALUES ('password', '', '');
--
GitLab