v0 protocol: use size_t for capability length/offset

When parsing server capabilities, we use "int" to store lengths and
offsets. At first glance this seems like a spot where our parser may be
confused by integer overflow if somebody sent us a malicious response.

In practice these strings are all bounded by the 64k limit of a
pkt-line, so using "int" is OK. However, it makes the code simpler to
audit if they just use size_t everywhere. Note that because we take
these parameters as pointers, this also forces many callers to update
their declared types.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This commit is contained in:
Jeff King 2023-04-14 17:25:20 -04:00 committed by Junio C Hamano
parent c4716236f2
commit 7ce4c8f752
7 changed files with 19 additions and 19 deletions

View File

@ -2093,7 +2093,7 @@ static struct command *read_head_info(struct packet_reader *reader,
const char *feature_list = reader->line + linelen + 1; const char *feature_list = reader->line + linelen + 1;
const char *hash = NULL; const char *hash = NULL;
const char *client_sid; const char *client_sid;
int len = 0; size_t len = 0;
if (parse_feature_request(feature_list, "report-status")) if (parse_feature_request(feature_list, "report-status"))
report_status = 1; report_status = 1;
if (parse_feature_request(feature_list, "report-status-v2")) if (parse_feature_request(feature_list, "report-status-v2"))

View File

@ -22,7 +22,7 @@
static char *server_capabilities_v1; static char *server_capabilities_v1;
static struct strvec server_capabilities_v2 = STRVEC_INIT; static struct strvec server_capabilities_v2 = STRVEC_INIT;
static const char *next_server_feature_value(const char *feature, int *len, int *offset); static const char *next_server_feature_value(const char *feature, size_t *len, size_t *offset);
static int check_ref(const char *name, unsigned int flags) static int check_ref(const char *name, unsigned int flags)
{ {
@ -205,10 +205,10 @@ reject:
static void annotate_refs_with_symref_info(struct ref *ref) static void annotate_refs_with_symref_info(struct ref *ref)
{ {
struct string_list symref = STRING_LIST_INIT_DUP; struct string_list symref = STRING_LIST_INIT_DUP;
int offset = 0; size_t offset = 0;
while (1) { while (1) {
int len; size_t len;
const char *val; const char *val;
val = next_server_feature_value("symref", &len, &offset); val = next_server_feature_value("symref", &len, &offset);
@ -231,7 +231,7 @@ static void annotate_refs_with_symref_info(struct ref *ref)
static void process_capabilities(struct packet_reader *reader, int *linelen) static void process_capabilities(struct packet_reader *reader, int *linelen)
{ {
const char *feat_val; const char *feat_val;
int feat_len; size_t feat_len;
const char *line = reader->line; const char *line = reader->line;
int nul_location = strlen(line); int nul_location = strlen(line);
if (nul_location == *linelen) if (nul_location == *linelen)
@ -596,10 +596,10 @@ struct ref **get_remote_refs(int fd_out, struct packet_reader *reader,
return list; return list;
} }
const char *parse_feature_value(const char *feature_list, const char *feature, int *lenp, int *offset) const char *parse_feature_value(const char *feature_list, const char *feature, size_t *lenp, size_t *offset)
{ {
const char *orig_start = feature_list; const char *orig_start = feature_list;
int len; size_t len;
if (!feature_list) if (!feature_list)
return NULL; return NULL;
@ -623,7 +623,7 @@ const char *parse_feature_value(const char *feature_list, const char *feature, i
} }
/* feature with a value (e.g., "agent=git/1.2.3") */ /* feature with a value (e.g., "agent=git/1.2.3") */
else if (*value == '=') { else if (*value == '=') {
int end; size_t end;
value++; value++;
end = strcspn(value, " \t\n"); end = strcspn(value, " \t\n");
@ -645,8 +645,8 @@ const char *parse_feature_value(const char *feature_list, const char *feature, i
int server_supports_hash(const char *desired, int *feature_supported) int server_supports_hash(const char *desired, int *feature_supported)
{ {
int offset = 0; size_t offset = 0;
int len; size_t len;
const char *hash; const char *hash;
hash = next_server_feature_value("object-format", &len, &offset); hash = next_server_feature_value("object-format", &len, &offset);
@ -670,12 +670,12 @@ int parse_feature_request(const char *feature_list, const char *feature)
return !!parse_feature_value(feature_list, feature, NULL, NULL); return !!parse_feature_value(feature_list, feature, NULL, NULL);
} }
static const char *next_server_feature_value(const char *feature, int *len, int *offset) static const char *next_server_feature_value(const char *feature, size_t *len, size_t *offset)
{ {
return parse_feature_value(server_capabilities_v1, feature, len, offset); return parse_feature_value(server_capabilities_v1, feature, len, offset);
} }
const char *server_feature_value(const char *feature, int *len) const char *server_feature_value(const char *feature, size_t *len)
{ {
return parse_feature_value(server_capabilities_v1, feature, len, NULL); return parse_feature_value(server_capabilities_v1, feature, len, NULL);
} }

View File

@ -12,14 +12,14 @@ int finish_connect(struct child_process *conn);
int git_connection_is_socket(struct child_process *conn); int git_connection_is_socket(struct child_process *conn);
int server_supports(const char *feature); int server_supports(const char *feature);
int parse_feature_request(const char *features, const char *feature); int parse_feature_request(const char *features, const char *feature);
const char *server_feature_value(const char *feature, int *len_ret); const char *server_feature_value(const char *feature, size_t *len_ret);
int url_is_local_not_ssh(const char *url); int url_is_local_not_ssh(const char *url);
struct packet_reader; struct packet_reader;
enum protocol_version discover_version(struct packet_reader *reader); enum protocol_version discover_version(struct packet_reader *reader);
int server_supports_hash(const char *desired, int *feature_supported); int server_supports_hash(const char *desired, int *feature_supported);
const char *parse_feature_value(const char *feature_list, const char *feature, int *lenp, int *offset); const char *parse_feature_value(const char *feature_list, const char *feature, size_t *lenp, size_t *offset);
int server_supports_v2(const char *c); int server_supports_v2(const char *c);
void ensure_server_supports_v2(const char *c); void ensure_server_supports_v2(const char *c);
int server_feature_v2(const char *c, const char **v); int server_feature_v2(const char *c, const char **v);

View File

@ -1099,7 +1099,7 @@ static struct ref *do_fetch_pack(struct fetch_pack_args *args,
struct ref *ref = copy_ref_list(orig_ref); struct ref *ref = copy_ref_list(orig_ref);
struct object_id oid; struct object_id oid;
const char *agent_feature; const char *agent_feature;
int agent_len; size_t agent_len;
struct fetch_negotiator negotiator_alloc; struct fetch_negotiator negotiator_alloc;
struct fetch_negotiator *negotiator; struct fetch_negotiator *negotiator;
@ -1117,7 +1117,7 @@ static struct ref *do_fetch_pack(struct fetch_pack_args *args,
agent_supported = 1; agent_supported = 1;
if (agent_len) if (agent_len)
print_verbose(args, _("Server version is %.*s"), print_verbose(args, _("Server version is %.*s"),
agent_len, agent_feature); (int)agent_len, agent_feature);
} }
if (!server_supports("session-id")) if (!server_supports("session-id"))

View File

@ -538,7 +538,7 @@ int send_pack(struct send_pack_args *args,
die(_("the receiving end does not support this repository's hash algorithm")); die(_("the receiving end does not support this repository's hash algorithm"));
if (args->push_cert != SEND_PACK_PUSH_CERT_NEVER) { if (args->push_cert != SEND_PACK_PUSH_CERT_NEVER) {
int len; size_t len;
push_cert_nonce = server_feature_value("push-cert", &len); push_cert_nonce = server_feature_value("push-cert", &len);
if (push_cert_nonce) { if (push_cert_nonce) {
reject_invalid_nonce(push_cert_nonce, len); reject_invalid_nonce(push_cert_nonce, len);

View File

@ -317,7 +317,7 @@ static struct ref *handshake(struct transport *transport, int for_push,
struct git_transport_data *data = transport->data; struct git_transport_data *data = transport->data;
struct ref *refs = NULL; struct ref *refs = NULL;
struct packet_reader reader; struct packet_reader reader;
int sid_len; size_t sid_len;
const char *server_sid; const char *server_sid;
connect_setup(transport, for_push); connect_setup(transport, for_push);

View File

@ -1067,7 +1067,7 @@ static void receive_needs(struct upload_pack_data *data,
const char *features; const char *features;
struct object_id oid_buf; struct object_id oid_buf;
const char *arg; const char *arg;
int feature_len; size_t feature_len;
reset_timeout(data->timeout); reset_timeout(data->timeout);
if (packet_reader_read(reader) != PACKET_READ_NORMAL) if (packet_reader_read(reader) != PACKET_READ_NORMAL)