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
This commit is contained in:
rl1987 2018-05-23 11:44:43 +02:00 committed by Nick Mathewson
parent 01cf3007b5
commit 58cb2ed243
2 changed files with 13 additions and 15 deletions

View File

@ -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);
}

View File

@ -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 */