Skip to content

Commit 7ce4c8f

Browse files
peffgitster
authored andcommitted
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 <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent c471623 commit 7ce4c8f

File tree

7 files changed

+19
-19
lines changed

7 files changed

+19
-19
lines changed

builtin/receive-pack.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2093,7 +2093,7 @@ static struct command *read_head_info(struct packet_reader *reader,
20932093
const char *feature_list = reader->line + linelen + 1;
20942094
const char *hash = NULL;
20952095
const char *client_sid;
2096-
int len = 0;
2096+
size_t len = 0;
20972097
if (parse_feature_request(feature_list, "report-status"))
20982098
report_status = 1;
20992099
if (parse_feature_request(feature_list, "report-status-v2"))

connect.c

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@
2222

2323
static char *server_capabilities_v1;
2424
static struct strvec server_capabilities_v2 = STRVEC_INIT;
25-
static const char *next_server_feature_value(const char *feature, int *len, int *offset);
25+
static const char *next_server_feature_value(const char *feature, size_t *len, size_t *offset);
2626

2727
static int check_ref(const char *name, unsigned int flags)
2828
{
@@ -205,10 +205,10 @@ static void parse_one_symref_info(struct string_list *symref, const char *val, i
205205
static void annotate_refs_with_symref_info(struct ref *ref)
206206
{
207207
struct string_list symref = STRING_LIST_INIT_DUP;
208-
int offset = 0;
208+
size_t offset = 0;
209209

210210
while (1) {
211-
int len;
211+
size_t len;
212212
const char *val;
213213

214214
val = next_server_feature_value("symref", &len, &offset);
@@ -231,7 +231,7 @@ static void annotate_refs_with_symref_info(struct ref *ref)
231231
static void process_capabilities(struct packet_reader *reader, int *linelen)
232232
{
233233
const char *feat_val;
234-
int feat_len;
234+
size_t feat_len;
235235
const char *line = reader->line;
236236
int nul_location = strlen(line);
237237
if (nul_location == *linelen)
@@ -596,10 +596,10 @@ struct ref **get_remote_refs(int fd_out, struct packet_reader *reader,
596596
return list;
597597
}
598598

599-
const char *parse_feature_value(const char *feature_list, const char *feature, int *lenp, int *offset)
599+
const char *parse_feature_value(const char *feature_list, const char *feature, size_t *lenp, size_t *offset)
600600
{
601601
const char *orig_start = feature_list;
602-
int len;
602+
size_t len;
603603

604604
if (!feature_list)
605605
return NULL;
@@ -623,7 +623,7 @@ const char *parse_feature_value(const char *feature_list, const char *feature, i
623623
}
624624
/* feature with a value (e.g., "agent=git/1.2.3") */
625625
else if (*value == '=') {
626-
int end;
626+
size_t end;
627627

628628
value++;
629629
end = strcspn(value, " \t\n");
@@ -645,8 +645,8 @@ const char *parse_feature_value(const char *feature_list, const char *feature, i
645645

646646
int server_supports_hash(const char *desired, int *feature_supported)
647647
{
648-
int offset = 0;
649-
int len;
648+
size_t offset = 0;
649+
size_t len;
650650
const char *hash;
651651

652652
hash = next_server_feature_value("object-format", &len, &offset);
@@ -670,12 +670,12 @@ int parse_feature_request(const char *feature_list, const char *feature)
670670
return !!parse_feature_value(feature_list, feature, NULL, NULL);
671671
}
672672

673-
static const char *next_server_feature_value(const char *feature, int *len, int *offset)
673+
static const char *next_server_feature_value(const char *feature, size_t *len, size_t *offset)
674674
{
675675
return parse_feature_value(server_capabilities_v1, feature, len, offset);
676676
}
677677

678-
const char *server_feature_value(const char *feature, int *len)
678+
const char *server_feature_value(const char *feature, size_t *len)
679679
{
680680
return parse_feature_value(server_capabilities_v1, feature, len, NULL);
681681
}

connect.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,14 +12,14 @@ int finish_connect(struct child_process *conn);
1212
int git_connection_is_socket(struct child_process *conn);
1313
int server_supports(const char *feature);
1414
int parse_feature_request(const char *features, const char *feature);
15-
const char *server_feature_value(const char *feature, int *len_ret);
15+
const char *server_feature_value(const char *feature, size_t *len_ret);
1616
int url_is_local_not_ssh(const char *url);
1717

1818
struct packet_reader;
1919
enum protocol_version discover_version(struct packet_reader *reader);
2020

2121
int server_supports_hash(const char *desired, int *feature_supported);
22-
const char *parse_feature_value(const char *feature_list, const char *feature, int *lenp, int *offset);
22+
const char *parse_feature_value(const char *feature_list, const char *feature, size_t *lenp, size_t *offset);
2323
int server_supports_v2(const char *c);
2424
void ensure_server_supports_v2(const char *c);
2525
int server_feature_v2(const char *c, const char **v);

fetch-pack.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1099,7 +1099,7 @@ static struct ref *do_fetch_pack(struct fetch_pack_args *args,
10991099
struct ref *ref = copy_ref_list(orig_ref);
11001100
struct object_id oid;
11011101
const char *agent_feature;
1102-
int agent_len;
1102+
size_t agent_len;
11031103
struct fetch_negotiator negotiator_alloc;
11041104
struct fetch_negotiator *negotiator;
11051105

@@ -1117,7 +1117,7 @@ static struct ref *do_fetch_pack(struct fetch_pack_args *args,
11171117
agent_supported = 1;
11181118
if (agent_len)
11191119
print_verbose(args, _("Server version is %.*s"),
1120-
agent_len, agent_feature);
1120+
(int)agent_len, agent_feature);
11211121
}
11221122

11231123
if (!server_supports("session-id"))

send-pack.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -538,7 +538,7 @@ int send_pack(struct send_pack_args *args,
538538
die(_("the receiving end does not support this repository's hash algorithm"));
539539

540540
if (args->push_cert != SEND_PACK_PUSH_CERT_NEVER) {
541-
int len;
541+
size_t len;
542542
push_cert_nonce = server_feature_value("push-cert", &len);
543543
if (push_cert_nonce) {
544544
reject_invalid_nonce(push_cert_nonce, len);

transport.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -317,7 +317,7 @@ static struct ref *handshake(struct transport *transport, int for_push,
317317
struct git_transport_data *data = transport->data;
318318
struct ref *refs = NULL;
319319
struct packet_reader reader;
320-
int sid_len;
320+
size_t sid_len;
321321
const char *server_sid;
322322

323323
connect_setup(transport, for_push);

upload-pack.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1067,7 +1067,7 @@ static void receive_needs(struct upload_pack_data *data,
10671067
const char *features;
10681068
struct object_id oid_buf;
10691069
const char *arg;
1070-
int feature_len;
1070+
size_t feature_len;
10711071

10721072
reset_timeout(data->timeout);
10731073
if (packet_reader_read(reader) != PACKET_READ_NORMAL)

0 commit comments

Comments
 (0)