Skip to content
Permalink
Browse files Browse the repository at this point in the history
SECURITY: Fix DoS on overly long input from Postfix
Thanks to Mateusz Jończyk who reported this issue and gave valuable
feedback for its resolution.

PostSRSd would hang on an overly long GET request, because the
fread()/fwrite() logic in the subprocess would get confused by the
remaining input line in its buffer.

Theoretically, this error should never occur, as Postfix is supposed to
send valid email addresses only, which are shorter than the buffer, even
assuming every single character is percent-encoded. However, Postfix
sometimes does seem to send malformed request with multiple concatenated
email addresses. I'm not sure if there's a reliable way to trigger this
condition by an external attacker, but it is a security bug in PostSRSd
nevertheless.
  • Loading branch information
roehling committed Mar 21, 2021
1 parent 03537f9 commit 077be98
Show file tree
Hide file tree
Showing 2 changed files with 69 additions and 27 deletions.
56 changes: 33 additions & 23 deletions postsrsd.c
Expand Up @@ -645,7 +645,7 @@ int main(int argc, char **argv)
while (TRUE)
{
int conn;
FILE *fp;
FILE *fp_read, *fp_write;
char linebuf[1024], *line;
char keybuf[1024], *key;

Expand Down Expand Up @@ -674,47 +674,57 @@ int main(int argc, char **argv)
* daemon process from restarting */
for (i = 0; i < socket_count; ++i)
close(sockets[i]);

fp = fdopen(conn, "r+");
if (fp == NULL)
exit(EXIT_FAILURE);
fds[0].fd = conn;
fds[0].events = POLLIN;
if (poll(fds, 1, timeout * 1000) <= 0)
/* create separate input/output streams */
fp_read = fdopen(conn, "r");
if (fp_read == NULL)
return EXIT_FAILURE;
fp_write = fdopen(dup(conn), "w");
if (fp_write == NULL)
return EXIT_FAILURE;
line = fgets(linebuf, sizeof(linebuf), fp);
while (line)
errno = 0;
alarm(timeout);
if (errno != 0)
return EXIT_FAILURE;
while ((line = fgets(linebuf, sizeof(linebuf), fp_read)))
{
fseek(fp, 0, SEEK_CUR); /* Workaround for Solaris */
char *token;
alarm(0);
if (strlen(line) >= sizeof(linebuf) - 1)
{
fprintf(fp_write, "500 Invalid request\n");
fflush(fp_write);
return EXIT_FAILURE;
}
token = strtok(line, " \r\n");
if (token == NULL || strcmp(token, "get") != 0)
{
fprintf(fp, "500 Invalid request\n");
fflush(fp);
fprintf(fp_write, "500 Invalid request\n");
fflush(fp_write);
return EXIT_FAILURE;
}
token = strtok(NULL, "\r\n");
if (!token)
{
fprintf(fp, "500 Invalid request\n");
fflush(fp);
fprintf(fp_write, "500 Invalid request\n");
fflush(fp_write);
return EXIT_FAILURE;
}
key = url_decode(keybuf, sizeof(keybuf), token);
if (!key)
{
fprintf(fp, "500 Invalid request\n");
fflush(fp);
fprintf(fp_write, "500 Invalid request\n");
fflush(fp_write);
return EXIT_FAILURE;
}
handler[sc](srs, fp, key, domain, excludes);
fflush(fp);
if (poll(fds, 1, timeout * 1000) <= 0)
break;
line = fgets(linebuf, sizeof(linebuf), fp);
handler[sc](srs, fp_write, key, domain, excludes);
fflush(fp_write);
errno = 0;
alarm(timeout);
if (errno != 0)
return EXIT_FAILURE;
}
fclose(fp);
fclose(fp_write);
fclose(fp_read);
return EXIT_SUCCESS;
}
close(conn);
Expand Down
40 changes: 36 additions & 4 deletions run_postsrsd_tests.bats
Expand Up @@ -3,7 +3,7 @@

if [ ! -x "$POSTSRSD" ]
then
for builddir in . build* obj*
for builddir in . build* obj* _build*
do
if [ -x "${builddir}/postsrsd" ]
then
Expand All @@ -15,7 +15,7 @@ fi
if [ ! -x "$POSTSRSD" ]
then
cat>&2 <<- EOF
cannot find postsrsd executable (looked in ., build*, obj*)
cannot find postsrsd executable (looked in ., build*, obj*, _build*)
please build the executable first, or set the POSTSRSD
environment variable if it is in a different location.
Expand All @@ -26,12 +26,19 @@ fi
LANG=C.UTF-8


fillchar()
{
local count="$1"
local char="$2"
eval 'printf "'"$char"'%.0s" {1..'"$count"'}'
}

start_postsrsd_at()
{
echo 'tops3cr3t' > "$BATS_TMPDIR/postsrsd.secret"
local faketime="$1"
shift
faketime "${faketime}" ${POSTSRSD} -D -f 10001 -r 10002 -p "$BATS_TMPDIR/postsrsd.pid" -s "$BATS_TMPDIR/postsrsd.secret" -d example.com "$@"
faketime "${faketime}" ${POSTSRSD} -D -t1 -f 10001 -r 10002 -p "$BATS_TMPDIR/postsrsd.pid" -s "$BATS_TMPDIR/postsrsd.secret" -d example.com "$@"
}

stop_postsrsd()
Expand Down Expand Up @@ -159,7 +166,7 @@ teardown()
[[ "$line" =~ ^"500 Domain excluded" ]]
}

@test "SRS invalid requests" {
@test "Malformed or invalid requests" {
start_postsrsd_at "2020-01-01 00:01:00 UTC"
exec 9<>/dev/tcp/127.0.0.1/10001
echo>&9 "get"
Expand All @@ -173,4 +180,29 @@ teardown()
echo>&9 "get encoding%error@otherdomain.com"
read<&9 line
[[ "$line" =~ ^500 ]]
exec 9<>/dev/tcp/127.0.0.1/10001
# Try to overflow the input buffer
echo>&9 "get too_long@`fillchar 1024 a`.com"
read<&9 line
[[ "$line" =~ ^500 ]]
}

@test "Pipelining multiple requests" {
start_postsrsd_at "2020-01-01 00:01:00 UTC"
exec 9<>/dev/tcp/127.0.0.1/10001
# Send two requests at once and see if PostSRSd answers both
echo>&9 -e "get test@domain1.com\nget test@domain2.com"
read<&9 line
[[ "$line" =~ ^200 ]]
read<&9 line
[[ "$line" =~ ^200 ]]
}

@test "Session timeout" {
start_postsrsd_at "2020-01-01 00:01:00 UTC"
exec 9<>/dev/tcp/127.0.0.1/10001
# Wait until PostSRSd disconnects due to inactivity
sleep 2
echo >&9 "get test@example.com"
! read <&9 line
}

0 comments on commit 077be98

Please sign in to comment.