Hi,
I noticed that pg_saslprep() in src/common/saslprep.c performs the prohibited
code point and bidirectional checks on input_chars rather than output_chars,
even though NFKC normalization has already been performed.
Since NFKC normalization can change both the code point values and the number
of code points, this can lead to incorrect results. The existing comments in
pg_saslprep() explicitly describe checking the output string after
normalization, suggesting that the current behavior is unintentional.
I found two examples that demonstrate the problem:
1. U+0061 followed by U+0340 normalizes to U+00E0, which is valid. The current
implementation returns SASLPREP_PROHIBITED instead of SASLPREP_SUCCESS.
2. U+FB1D normalizes to U+05D9 followed by U+05B4. The normalized string
violates the bidirectional rules because it does not end with a RandALCat
character. The current implementation incorrectly returns SASLPREP_SUCCESS.
While investigating this issue, I also verified all the code point tables used
by pg_saslprep() against RFC 3454 and found no discrepancies.
After building and installing the test_saslprep module from
src/test/modules/test_saslprep, the issue can be reproduced in a running
Postgres instance using:
CREATE EXTENSION IF NOT EXISTS test_saslprep;
WITH cases (name, src, expected_status, expected_res) AS (
VALUES
('step3', decode('61cd80', 'hex'), 'SUCCESS', decode('c3a0', 'hex')),
('step4', decode('efac9d', 'hex'), 'PROHIBITED', NULL::bytea)
)
SELECT name, status, encode(res, 'hex') AS result,
status = expected_status AND res IS NOT DISTINCT FROM expected_res AS
passed
FROM cases
CROSS JOIN LATERAL test_saslprep(src);
On the unmodified master branch, both cases return false in the "passed"
column. With the patch applied, both return true.
The attached patch changes the prohibited code point and bidirectional checks
to operate on the normalized output, rather than the pre-normalization input.
It also rejects an empty result after normalization.
The patch includes regression tests for both cases. These tests fail on the
unmodified master branch and pass with the patch applied.
Testing was performed with a clean build configured with --enable-tap-tests,
--enable-cassert, and --enable-debug. All applicable check-world tests passed
with PG_TEST_EXTRA=saslprep, including the additional SASLprep TAP tests and
the authentication tests.
The change is intended to correct the existing SASLprep implementation
according to RFC 3454 and RFC 4013.
Since Postgres falls back to the original password when SASLprep fails, this
change may affect the input to SCRAM key derivation for some passwords. This
could cause compatibility issues with existing SCRAM verifiers or between
different Postgres versions.
If retaining the current behavior is preferred for compatibility reasons, I
think it would be useful to document this explicitly in the code, since the
current implementation appears to contradict the processing order specified by
RFC 3454.
I would appreciate your review and thoughts on the proposed change,
particularly regarding the potential compatibility implications.
Regards,
Robert I NilssonFrom b8ac9a387069d74ea106560b8c52b3e9d5561153 Mon Sep 17 00:00:00 2001
From: Robert I Nilsson <[email protected]>
Date: Tue, 10 Nov 2026 17:36:05 +0100
Subject: [PATCH v1] Fix SASLprep validation after NFKC normalization
The prohibited code point and bidirectional checks in pg_saslprep()
are performed on the original input rather than the NFKC-normalized
output. Since normalization can change both the code points and
their number, this can lead to valid strings being rejected and
invalid strings being accepted.
Perform these checks on the normalized output, using its length
instead of the original input length. Also reject an empty result
after normalization.
Add regression tests covering both incorrect rejection of a valid
string and incorrect acceptance of a string violating the
bidirectional rules.
---
src/common/saslprep.c | 24 ++++++++++++-------
.../test_saslprep/expected/test_saslprep.out | 21 ++++++++++++++++
.../test_saslprep/sql/test_saslprep.sql | 16 +++++++++++++
3 files changed, 53 insertions(+), 8 deletions(-)
diff --git a/src/common/saslprep.c b/src/common/saslprep.c
index a2bb60ac0cb..475d98bfe4b 100644
--- a/src/common/saslprep.c
+++ b/src/common/saslprep.c
@@ -1049,6 +1049,7 @@ pg_saslprep(const char *input, char **output)
char32_t *input_chars = NULL;
char32_t *output_chars = NULL;
int input_size;
+ int output_size;
char *result;
int result_size;
int count;
@@ -1121,13 +1122,20 @@ pg_saslprep(const char *input, char **output)
if (!output_chars)
goto oom;
+ output_size = 0;
+ for (wp = output_chars; *wp; wp++)
+ output_size++;
+
+ if (output_size == 0)
+ goto prohibited; /* don't allow empty password, after NFKC */
+
/*
* 3) Prohibit -- Check for any characters that are not allowed in the
* output. If any are found, return an error.
*/
- for (i = 0; i < input_size; i++)
+ for (i = 0; i < output_size; i++)
{
- char32_t code = input_chars[i];
+ char32_t code = output_chars[i];
if (IS_CODE_IN_TABLE(code, prohibited_output_ranges))
goto prohibited;
@@ -1157,9 +1165,9 @@ pg_saslprep(const char *input, char **output)
* MUST be the last character of the string."
*/
contains_RandALCat = false;
- for (i = 0; i < input_size; i++)
+ for (i = 0; i < output_size; i++)
{
- char32_t code = input_chars[i];
+ char32_t code = output_chars[i];
if (IS_CODE_IN_TABLE(code, RandALCat_codepoint_ranges))
{
@@ -1170,12 +1178,12 @@ pg_saslprep(const char *input, char **output)
if (contains_RandALCat)
{
- char32_t first = input_chars[0];
- char32_t last = input_chars[input_size - 1];
+ char32_t first = output_chars[0];
+ char32_t last = output_chars[output_size - 1];
- for (i = 0; i < input_size; i++)
+ for (i = 0; i < output_size; i++)
{
- char32_t code = input_chars[i];
+ char32_t code = output_chars[i];
if (IS_CODE_IN_TABLE(code, LCat_codepoint_ranges))
goto prohibited;
diff --git a/src/test/modules/test_saslprep/expected/test_saslprep.out b/src/test/modules/test_saslprep/expected/test_saslprep.out
index 92f93365343..a3b75d45d33 100644
--- a/src/test/modules/test_saslprep/expected/test_saslprep.out
+++ b/src/test/modules/test_saslprep/expected/test_saslprep.out
@@ -149,4 +149,25 @@ SELECT
<DEL> | \x7f | (,PROHIBITED)
(128 rows)
+-- Test prohibited code points and bidi rules after normalization
+WITH cases(name, src, expected_status, expected_res) AS (
+ VALUES
+ ('step3', decode('61cd80', 'hex'),
+ 'SUCCESS', decode('c3a0', 'hex')),
+ ('step4', decode('efac9d', 'hex'),
+ 'PROHIBITED', NULL::bytea)
+)
+SELECT name,
+ status,
+ encode(res, 'hex') AS result,
+ status = expected_status
+ AND res IS NOT DISTINCT FROM expected_res AS passed
+FROM cases
+CROSS JOIN LATERAL test_saslprep(src);
+ name | status | result | passed
+-------+------------+--------+--------
+ step3 | SUCCESS | c3a0 | t
+ step4 | PROHIBITED | | t
+(2 rows)
+
DROP EXTENSION test_saslprep;
diff --git a/src/test/modules/test_saslprep/sql/test_saslprep.sql b/src/test/modules/test_saslprep/sql/test_saslprep.sql
index 00bad48eca7..c2ddd7dda42 100644
--- a/src/test/modules/test_saslprep/sql/test_saslprep.sql
+++ b/src/test/modules/test_saslprep/sql/test_saslprep.sql
@@ -16,4 +16,20 @@ SELECT
test_saslprep(set_byte('\x00'::bytea, 0, a)) AS saslprep
FROM generate_series(0,127) AS a;
+-- Test prohibited code points and bidi rules after normalization
+WITH cases(name, src, expected_status, expected_res) AS (
+ VALUES
+ ('step3', decode('61cd80', 'hex'),
+ 'SUCCESS', decode('c3a0', 'hex')),
+ ('step4', decode('efac9d', 'hex'),
+ 'PROHIBITED', NULL::bytea)
+)
+SELECT name,
+ status,
+ encode(res, 'hex') AS result,
+ status = expected_status
+ AND res IS NOT DISTINCT FROM expected_res AS passed
+FROM cases
+CROSS JOIN LATERAL test_saslprep(src);
+
DROP EXTENSION test_saslprep;
--
2.53.0
