[tor-commits] [tor/master] Fix various issues pointed out by Nick and Andrea.
nickm at torproject.org
nickm at torproject.org
Tue Mar 19 17:25:51 UTC 2013
commit b5dceab1751dfa12b27b3042a49d90e0b02c2e0c
Author: George Kadianakis <desnacked at riseup.net>
Date: Sat Feb 9 18:46:10 2013 +0000
Fix various issues pointed out by Nick and Andrea.
- Document the key=value format.
- Constify equal_sign_pos.
- Pass some strings that are about to be logged to escape().
- Update documentation and fix some bugs in tor_escape_str_for_socks_arg().
- Use string_is_key_value() in parse_bridge_line().
- Parenthesize a forgotten #define
- Add some more comments.
- Add some more unit test cases.
---
src/common/util.c | 27 +++++++++++++++------------
src/or/config.c | 11 ++++++-----
src/or/connection.c | 7 ++++++-
src/test/test_util.c | 14 ++++++++++++--
4 files changed, 39 insertions(+), 20 deletions(-)
diff --git a/src/common/util.c b/src/common/util.c
index b2f12bf..9aba7d6 100644
--- a/src/common/util.c
+++ b/src/common/util.c
@@ -865,30 +865,30 @@ tor_digest_is_zero(const char *digest)
return tor_memeq(digest, ZERO_DIGEST, DIGEST_LEN);
}
-/** Return true if <b>string</b> is a valid '<key>=<value>' string.
+/** Return true if <b>string</b> is a valid '<key>=[<value>]' string.
* <value> is optional, to indicate the empty string. */
int
string_is_key_value(const char *string)
{
/* position of equal sign in string */
- char *equal_sign_pos = NULL;
+ const char *equal_sign_pos = NULL;
tor_assert(string);
- if (strlen(string) < 2) { /* "x=a" is shortest args string */
- log_warn(LD_GENERAL, "'%s' is too short to be a k=v value.", string);
+ if (strlen(string) < 2) { /* "x=" is shortest args string */
+ log_warn(LD_GENERAL, "'%s' is too short to be a k=v value.", escaped(string));
return 0;
}
equal_sign_pos = strchr(string, '=');
if (!equal_sign_pos) {
- log_warn(LD_GENERAL, "'%s' is not a k=v value.", string);
+ log_warn(LD_GENERAL, "'%s' is not a k=v value.", escaped(string));
return 0;
}
/* validate that the '=' is not in the beginning of the string. */
if (equal_sign_pos == string) {
- log_warn(LD_GENERAL, "'%s' is not a valid k=v value.", string);
+ log_warn(LD_GENERAL, "'%s' is not a valid k=v value.", escaped(string));
return 0;
}
@@ -1279,9 +1279,10 @@ wrap_string(smartlist_t *out, const char *string, size_t width,
}
}
-/** Escape every character of <b>string</b> that belongs to the set of
- * characters <b>set</b>. Use <b>escape_char</b> as the character to
- * use for escaping. */
+/** Escape every ";" or "\" character of <b>string</b>. Use
+ * <b>escape_char</b> as the character to use for escaping.
+ * The returned string is allocated on the heap and it's the
+ * responsibility of the caller to free it. */
char *
tor_escape_str_for_socks_arg(const char *string)
{
@@ -1294,8 +1295,8 @@ tor_escape_str_for_socks_arg(const char *string)
length = strlen(string);
- if (!length)
- return NULL;
+ if (!length) /* If we were given the empty string, return the same. */
+ return tor_strdup("");
/* (new_length > SIZE_MAX) => ((length * 2) + 1 > SIZE_MAX) =>
(length*2 > SIZE_MAX - 1) => (length > (SIZE_MAX - 1)/2) */
if (length > (SIZE_MAX - 1)/2) /* check for overflow */
@@ -1304,7 +1305,7 @@ tor_escape_str_for_socks_arg(const char *string)
/* this should be enough even if all characters must be escaped */
new_length = (length * 2) + 1;
- new_string = new_cp = tor_malloc_zero(new_length);
+ new_string = new_cp = tor_malloc(new_length);
while (*string) {
if (strchr(chars_to_escape, *string))
@@ -1313,6 +1314,8 @@ tor_escape_str_for_socks_arg(const char *string)
*new_cp++ = *string++;
}
+ *new_cp = '\0'; /* NUL-terminate the new string */
+
return new_string;
}
diff --git a/src/or/config.c b/src/or/config.c
index d057dd8..a09dda9 100644
--- a/src/or/config.c
+++ b/src/or/config.c
@@ -2875,14 +2875,14 @@ options_validate(or_options_t *old_options, or_options_t *options,
size_t len;
len = strlen(options->Socks5ProxyUsername);
- if (len < 1 || len > 255)
+ if (len < 1 || len > MAX_SOCKS5_AUTH_FIELD_SIZE)
REJECT("Socks5ProxyUsername must be between 1 and 255 characters.");
if (!options->Socks5ProxyPassword)
REJECT("Socks5ProxyPassword must be included with Socks5ProxyUsername.");
len = strlen(options->Socks5ProxyPassword);
- if (len < 1 || len > 255)
+ if (len < 1 || len > MAX_SOCKS5_AUTH_FIELD_SIZE)
REJECT("Socks5ProxyPassword must be between 1 and 255 characters.");
} else if (options->Socks5ProxyPassword)
REJECT("Socks5ProxyPassword must be included with Socks5ProxyUsername.");
@@ -4120,11 +4120,12 @@ parse_bridge_line(const char *line, int validate_only)
field = smartlist_get(items, 0);
smartlist_del_keeporder(items, 0);
- /* If '=', it's a k=v value pair. */
- if (strchr(field, '=')) {
+ /* If it's a key=value pair, then it's a SOCKS argument for the
+ transport proxy... */
+ if (string_is_key_value(field)) {
socks_args = smartlist_new();
smartlist_add(socks_args, field);
- } else { /* If no '=', it's the fingerprint. */
+ } else { /* ...otherwise, it's the bridge fingerprint. */
fingerprint = field;
}
diff --git a/src/or/connection.c b/src/or/connection.c
index 6bac59b..b0fbe52 100644
--- a/src/or/connection.c
+++ b/src/or/connection.c
@@ -1585,7 +1585,7 @@ get_proxy_type(void)
/* One byte for the version, one for the command, two for the
port, and four for the addr... and, one more for the
username NUL: */
-#define SOCKS4_STANDARD_BUFFER_SIZE 1 + 1 + 2 + 4 + 1
+#define SOCKS4_STANDARD_BUFFER_SIZE (1 + 1 + 2 + 4 + 1)
/** Write a proxy request of <b>type</b> (socks4, socks5, https) to conn
* for conn->addr:conn->port, authenticating with the auth details given
@@ -1688,6 +1688,9 @@ connection_proxy_connect(connection_t *conn, int type)
memcpy(buf + 2, &portn, 2); /* port */
memcpy(buf + 4, &ip4addr, 4); /* addr */
+ /* Next packet field is the userid. If we have pluggable
+ transport SOCKS arguments, we have to embed them
+ there. Otherwise, we use an empty userid. */
if (socks_args_string) { /* place the SOCKS args string: */
tor_assert(strlen(socks_args_string) > 0);
tor_assert(buf_size >=
@@ -1951,6 +1954,8 @@ connection_read_proxy_handshake(connection_t *conn)
break;
}
+ /* Username and password lengths should have been checked
+ above and during torrc parsing. */
tor_assert(usize <= MAX_SOCKS5_AUTH_FIELD_SIZE &&
psize <= MAX_SOCKS5_AUTH_FIELD_SIZE);
reqsize = 3 + usize + psize;
diff --git a/src/test/test_util.c b/src/test/test_util.c
index b41f235..a307a79 100644
--- a/src/test/test_util.c
+++ b/src/test/test_util.c
@@ -813,9 +813,11 @@ test_util_escape_string_socks(void)
test_streq(escaped_string, "First rule: Do not use \\;");
tor_free(escaped_string);
- /** Ilegal: Empty string. */
+ /** Empty string. */
escaped_string = tor_escape_str_for_socks_arg("");
- test_assert(!escaped_string);
+ test_assert(escaped_string);
+ test_streq(escaped_string, "");
+ tor_free(escaped_string);
/** Escape all characters. */
escaped_string = tor_escape_str_for_socks_arg(";\\;\\");
@@ -823,6 +825,11 @@ test_util_escape_string_socks(void)
test_streq(escaped_string, "\\;\\\\\\;\\\\");
tor_free(escaped_string);
+ escaped_string = tor_escape_str_for_socks_arg(";");
+ test_assert(escaped_string);
+ test_streq(escaped_string, "\\;");
+ tor_free(escaped_string);
+
done:
tor_free(escaped_string);
}
@@ -834,7 +841,10 @@ test_util_string_is_key_value(void *ptr)
test_assert(string_is_key_value("key=value"));
test_assert(string_is_key_value("k=v"));
test_assert(string_is_key_value("key="));
+ test_assert(string_is_key_value("x="));
+ test_assert(string_is_key_value("xx="));
test_assert(!string_is_key_value("=value"));
+ test_assert(!string_is_key_value("=x"));
test_assert(!string_is_key_value("="));
/* ??? */
More information about the tor-commits
mailing list