Skip to content

Commit 40cd9d8

Browse files
Unique-Usmanttaylorr
authored andcommitted
parse: replace atoi() with strtoul_ui() and strtol_i()
Replace unsafe uses of atoi() with strtoul_ui() for unsigned integers and strtol_i() for signed integers across multiple files. This change improves error handling and prevents potential integer overflow issues. The following files were updated: - daemon.c: Update parsing of --timeout, --init-timeout, and --max-connections - imap-send.c: Improve parsing of UIDVALIDITY, UIDNEXT, APPENDUID, and tags - merge-ll.c: Enhance parsing of marker size in ll_merge and ll_merge_marker_size This change allows for better error detection when parsing integer values from command-line arguments and IMAP responses, making the code more robust and secure. This is a #leftoverbit discussed here: https://public-inbox.org/git/CAC4O8c-nuOTS=a0sVp1603KaM2bZjs+yNZzdAaa5CGTNGFE7hQ@mail.gmail.com/ Signed-off-by: Usman Akinyemi <[email protected]> Cc: [email protected] Cc: Patrick Steinhardt <[email protected]> Cc: [email protected] Cc: Christian Couder <[email protected]> Cc: Eric Sunshine <[email protected]> Cc: Taylor Blau <[email protected]> Signed-off-by: Taylor Blau <[email protected]>
1 parent 19c291e commit 40cd9d8

File tree

3 files changed

+19
-14
lines changed

3 files changed

+19
-14
lines changed

daemon.c

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1308,17 +1308,21 @@ int cmd_main(int argc, const char **argv)
13081308
continue;
13091309
}
13101310
if (skip_prefix(arg, "--timeout=", &v)) {
1311-
timeout = atoi(v);
1311+
if (strtoul_ui(v, 10, &timeout) < 0) {
1312+
die("'%s': not a valid integer for --timeout", v);
1313+
}
13121314
continue;
13131315
}
13141316
if (skip_prefix(arg, "--init-timeout=", &v)) {
1315-
init_timeout = atoi(v);
1317+
if (strtoul_ui(v, 10, &init_timeout) < 0) {
1318+
die("'%s': not a valid integer for --init-timeout", v);
1319+
}
13161320
continue;
13171321
}
13181322
if (skip_prefix(arg, "--max-connections=", &v)) {
1319-
max_connections = atoi(v);
1320-
if (max_connections < 0)
1321-
max_connections = 0; /* unlimited */
1323+
if (strtol_i(v, 10, &max_connections) != 0 || max_connections < 0) {
1324+
max_connections = 0; /* unlimited */
1325+
}
13221326
continue;
13231327
}
13241328
if (!strcmp(arg, "--strict-paths")) {

imap-send.c

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -668,12 +668,12 @@ static int parse_response_code(struct imap_store *ctx, struct imap_cmd_cb *cb,
668668
return RESP_BAD;
669669
}
670670
if (!strcmp("UIDVALIDITY", arg)) {
671-
if (!(arg = next_arg(&s)) || !(ctx->uidvalidity = atoi(arg))) {
671+
if (!(arg = next_arg(&s)) || strtol_i(arg, 10, &ctx->uidvalidity) != 0) {
672672
fprintf(stderr, "IMAP error: malformed UIDVALIDITY status\n");
673673
return RESP_BAD;
674674
}
675675
} else if (!strcmp("UIDNEXT", arg)) {
676-
if (!(arg = next_arg(&s)) || !(imap->uidnext = atoi(arg))) {
676+
if (!(arg = next_arg(&s)) || strtol_i(arg, 10, &imap->uidnext) != 0) {
677677
fprintf(stderr, "IMAP error: malformed NEXTUID status\n");
678678
return RESP_BAD;
679679
}
@@ -686,8 +686,8 @@ static int parse_response_code(struct imap_store *ctx, struct imap_cmd_cb *cb,
686686
for (; isspace((unsigned char)*p); p++);
687687
fprintf(stderr, "*** IMAP ALERT *** %s\n", p);
688688
} else if (cb && cb->ctx && !strcmp("APPENDUID", arg)) {
689-
if (!(arg = next_arg(&s)) || !(ctx->uidvalidity = atoi(arg)) ||
690-
!(arg = next_arg(&s)) || !(*(int *)cb->ctx = atoi(arg))) {
689+
if (!(arg = next_arg(&s)) || (strtol_i(arg, 10, &ctx->uidvalidity) != 0) ||
690+
!(arg = next_arg(&s)) || (strtol_i(arg, 10, (int *)cb->ctx) != 0)) {
691691
fprintf(stderr, "IMAP error: malformed APPENDUID status\n");
692692
return RESP_BAD;
693693
}
@@ -773,7 +773,10 @@ static int get_cmd_result(struct imap_store *ctx, struct imap_cmd *tcmd)
773773
if (!tcmd)
774774
return DRV_OK;
775775
} else {
776-
tag = atoi(arg);
776+
if (strtol_i(arg, 10, &tag) != 0) {
777+
fprintf(stderr, "IMAP error: malformed tag %s\n", arg);
778+
return RESP_BAD;
779+
}
777780
for (pcmdp = &imap->in_progress; (cmdp = *pcmdp); pcmdp = &cmdp->next)
778781
if (cmdp->tag == tag)
779782
goto gottag;

merge-ll.c

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -427,8 +427,7 @@ enum ll_merge_result ll_merge(mmbuffer_t *result_buf,
427427
git_check_attr(istate, path, check);
428428
ll_driver_name = check->items[0].value;
429429
if (check->items[1].value) {
430-
marker_size = atoi(check->items[1].value);
431-
if (marker_size <= 0)
430+
if (strtol_i(check->items[1].value, 10, &marker_size) != 0 || marker_size <= 0)
432431
marker_size = DEFAULT_CONFLICT_MARKER_SIZE;
433432
}
434433
driver = find_ll_merge_driver(ll_driver_name);
@@ -454,8 +453,7 @@ int ll_merge_marker_size(struct index_state *istate, const char *path)
454453
check = attr_check_initl("conflict-marker-size", NULL);
455454
git_check_attr(istate, path, check);
456455
if (check->items[0].value) {
457-
marker_size = atoi(check->items[0].value);
458-
if (marker_size <= 0)
456+
if (strtol_i(check->items[0].value, 10, &marker_size) != 0 || marker_size <= 0)
459457
marker_size = DEFAULT_CONFLICT_MARKER_SIZE;
460458
}
461459
return marker_size;

0 commit comments

Comments
 (0)