commit 58cb2ed24381d29458927c68e56aca0cae4b0f2e Author: rl1987 rl1987@sdf.lonestar.org Date: Wed May 23 11:44:43 2018 +0200
Fix buf_t advancement in fetch_buf_from_socks
We pullup 512 bytes of input to make sure that at least one SOCKS message ends up in head of linked list --- src/or/proto_socks.c | 24 +++++++++++------------- src/test/test_socks.c | 4 ++-- 2 files changed, 13 insertions(+), 15 deletions(-)
diff --git a/src/or/proto_socks.c b/src/or/proto_socks.c index 0c48d597e..9f6073d2a 100644 --- a/src/or/proto_socks.c +++ b/src/or/proto_socks.c @@ -32,8 +32,7 @@ static socks_result_t parse_socks(const char *data, socks_request_t *req, int log_sockstype, int safe_socks, - size_t *drain_out, - size_t *want_length_out); + size_t *drain_out); static int parse_socks_client(const uint8_t *data, size_t datalen, int state, char **reason, ssize_t *drain_out); @@ -125,7 +124,7 @@ parse_socks4_request(const uint8_t *raw_data, socks_request_t *req, goto end; } else if (parsed == -2) { res = SOCKS_RESULT_TRUNCATED; - if (datalen > 1024) { // XXX + if (datalen >= MAX_SOCKS_MESSAGE_LEN) { log_warn(LD_APP, "socks4: parsing failed - invalid request."); res = SOCKS_RESULT_INVALID; } @@ -273,7 +272,7 @@ parse_socks5_methods_request(const uint8_t *raw_data, socks_request_t *req, goto end; } else if (parsed == -2) { res = SOCKS_RESULT_TRUNCATED; - if (datalen > 1024) { // XXX + if (datalen > MAX_SOCKS_MESSAGE_LEN) { log_warn(LD_APP, "socks5: parsing failed - invalid version " "id/method selection message."); res = SOCKS_RESULT_INVALID; @@ -720,9 +719,9 @@ fetch_from_buf_socks(buf_t *buf, socks_request_t *req, int res = 0; size_t datalen = buf_datalen(buf); size_t n_drain; - size_t want_length = 128; const char *head = NULL; socks_result_t socks_res; + size_t n_pullup;
if (buf_datalen(buf) < 2) { /* version and another byte */ res = 0; @@ -731,13 +730,12 @@ fetch_from_buf_socks(buf_t *buf, socks_request_t *req,
do { n_drain = 0; - buf_pullup(buf, MAX(want_length, buf_datalen(buf)), - &head, &datalen); + n_pullup = MIN(MAX_SOCKS_MESSAGE_LEN, buf_datalen(buf)); + buf_pullup(buf, n_pullup, &head, &datalen); tor_assert(head && datalen >= 2); - want_length = 0;
socks_res = parse_socks(head, datalen, req, log_sockstype, - safe_socks, &n_drain, &want_length); + safe_socks, &n_drain);
if (socks_res == SOCKS_RESULT_INVALID) buf_clear(buf); @@ -752,6 +750,9 @@ fetch_from_buf_socks(buf_t *buf, socks_request_t *req, res = 1; break; case SOCKS_RESULT_TRUNCATED: + if (datalen == n_pullup) + return 0; + /* FALLTHRU */ case SOCKS_RESULT_MORE_EXPECTED: res = 0; break; @@ -812,14 +813,12 @@ static const char SOCKS_PROXY_IS_NOT_AN_HTTP_PROXY_MSG[] = * we'd like to see in the input buffer, if they're available. */ static int parse_socks(const char *data, size_t datalen, socks_request_t *req, - int log_sockstype, int safe_socks, size_t *drain_out, - size_t *want_length_out) + int log_sockstype, int safe_socks, size_t *drain_out) { uint8_t socksver;
if (datalen < 2) { /* We always need at least 2 bytes. */ - *want_length_out = 2; return 0; }
@@ -827,7 +826,6 @@ parse_socks(const char *data, size_t datalen, socks_request_t *req,
if (socksver == 5 || socksver == 4 || socksver == 1) { // XXX: RFC 1929 - *want_length_out = 128; // TODO remove this arg later return handle_socks_message((const uint8_t *)data, datalen, req, log_sockstype, safe_socks, drain_out); } diff --git a/src/test/test_socks.c b/src/test/test_socks.c index 9645ea32a..e1c5db6ee 100644 --- a/src/test/test_socks.c +++ b/src/test/test_socks.c @@ -142,7 +142,7 @@ test_socks_4_bad_arguments(void *ptr) get_options()->SafeSocks), OP_EQ, -1); buf_clear(buf); - expect_log_msg_containing("Port or DestIP is zero."); // !!! + expect_log_msg_containing("Port or DestIP is zero."); mock_clean_saved_logs();
/* Try with 0 port */ @@ -164,7 +164,7 @@ test_socks_4_bad_arguments(void *ptr) tt_int_op(fetch_from_buf_socks(buf, socks, 1, 0), OP_EQ, -1); buf_clear(buf); - expect_log_msg_containing("user name too long; rejecting."); + expect_log_msg_containing("socks4: parsing failed - invalid request."); mock_clean_saved_logs();
/* Try with 2000-byte hostname */