From 17bcfb2604ba3c20a78cdcc78f169b4db6927d25 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Tue, 8 Oct 2013 11:13:21 -0400 Subject: [PATCH 1/2] Raise buffer size, fix checks for format_exit_helper_status. This is probably not an exploitable bug, since you would need to have errno be a large negative value in the unix pluggable-transport launcher case. Still, best avoided. Fixes bug 9928; bugfix on 0.2.3.18-rc. --- changes/bug9928 | 5 +++++ src/common/util.c | 10 +++++----- 2 files changed, 10 insertions(+), 5 deletions(-) create mode 100644 changes/bug9928 diff --git a/changes/bug9928 b/changes/bug9928 new file mode 100644 index 0000000000..e19bcf463b --- /dev/null +++ b/changes/bug9928 @@ -0,0 +1,5 @@ + o Minor bugfixes: + - Avoid an off-by-one error when checking buffer boundaries when + formatting the exit status of a pluggable transport helper. + This is probably not an exploitable bug, but better safe than + sorry. Fixes bug 9928; bugfix on 0.2.3.18-rc. diff --git a/src/common/util.c b/src/common/util.c index 6fb597a3a5..5b0dbcd07e 100644 --- a/src/common/util.c +++ b/src/common/util.c @@ -3256,10 +3256,10 @@ format_hex_number_for_helper_exit_status(unsigned int x, char *buf, * hex_errno. Called between fork and _exit, so must be signal-handler * safe. * - * hex_errno must have at least HEX_ERRNO_SIZE bytes available. + * hex_errno must have at least HEX_ERRNO_SIZE+1 bytes available. * * The format of hex_errno is: "CHILD_STATE/ERRNO\n", left-padded - * with spaces. Note that there is no trailing \0. CHILD_STATE indicates where + * with spaces. CHILD_STATE indicates where * in the processs of starting the child process did the failure occur (see * CHILD_STATE_* macros for definition), and SAVED_ERRNO is the value of * errno when the failure occurred. @@ -3338,8 +3338,8 @@ format_helper_exit_status(unsigned char child_state, int saved_errno, left -= written; cur += written; - /* Check that we have enough space left for a newline */ - if (left <= 0) + /* Check that we have enough space left for a newline and a NUL */ + if (left <= 1) goto err; /* Emit the newline and NUL */ @@ -3594,7 +3594,7 @@ tor_spawn_background(const char *const filename, const char **argv, this is used for printing out the error message */ unsigned char child_state = CHILD_STATE_INIT; - char hex_errno[HEX_ERRNO_SIZE]; + char hex_errno[HEX_ERRNO_SIZE + 1]; static int max_fd = -1; From bfe56e05b08b940d432be9af824b969522eedc98 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Tue, 8 Oct 2013 12:06:06 -0400 Subject: [PATCH 2/2] Give credit to bug reporter for 9928 --- changes/bug9928 | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/changes/bug9928 b/changes/bug9928 index e19bcf463b..b72cea3d87 100644 --- a/changes/bug9928 +++ b/changes/bug9928 @@ -2,4 +2,5 @@ - Avoid an off-by-one error when checking buffer boundaries when formatting the exit status of a pluggable transport helper. This is probably not an exploitable bug, but better safe than - sorry. Fixes bug 9928; bugfix on 0.2.3.18-rc. + sorry. Fixes bug 9928; bugfix on 0.2.3.18-rc. Bug found by + Pedro Ribeiro.