trailer: use size_t for string offsets

Many of the string-parsing functions inside trailer.c return integer offsets into the string (e.g., to point to the end of the trailer block). Several of these use an "int" to return or store the offsets. On a system where "size_t" is much larger than "int" (e.g., most 64-bit ones), it's easy to feed a gigantic commit message that results in a negative offset. This can result in us reading memory before the string (if the int is used as an index) or far after (if it's implicitly cast to a size_t by passing to a strbuf function). Let's fix this by using size_t for all string offsets. Note that several of the functions need ssize_t, since they use "-1" as a sentinel value. The interactions here can be pretty subtle. E.g., end_of_title in find_trailer_start() does not itself need to be signed, but it is compared to the result of last_line(), which is. That promotes the latter to unsigned, and the ">=" does not behave as you might expect. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jeff King committed Aug 22, 2018 at 20:44 UTC 0d2db00e24ee2df4459151c5ba6de9306e30e727
1 file changed +20 -19
trailer.c
+20 -19
@@ -585,7 +585,7 @@ static const char *token_from_item(struct arg_item *item, char *tok)
585 return item->conf.name;
586 }
587
588 -static int token_matches_item(const char *tok, struct arg_item *item, int tok_len)
588 +static int token_matches_item(const char *tok, struct arg_item *item, size_t tok_len)
589 {
590 if (!strncasecmp(tok, item->conf.name, tok_len))
591 return 1;
@@ -603,7 +603,7 @@ static int token_matches_item(const char *tok, struct arg_item *item, int tok_le
603 * distinguished from the non-well-formed-line case (in which this function
604 * returns -1) because some callers of this function need such a distinction.
605 */
606 -static int find_separator(const char *line, const char *separators)
606 +static ssize_t find_separator(const char *line, const char *separators)
607 {
608 int whitespace_found = 0;
609 const char *c;
@@ -630,10 +630,10 @@ static int find_separator(const char *line, const char *separators)
630 */
631 static void parse_trailer(struct strbuf *tok, struct strbuf *val,
632 const struct conf_info **conf, const char *trailer,
633 - int separator_pos)
633 + ssize_t separator_pos)
634 {
635 struct arg_item *item;
636 - int tok_len;
636 + size_t tok_len;
637 struct list_head *pos;
638
639 if (separator_pos != -1) {
@@ -721,7 +721,7 @@ static void process_command_line_args(struct list_head *arg_head,
721 list_for_each(pos, new_trailer_head) {
722 struct new_trailer_item *tr =
723 list_entry(pos, struct new_trailer_item, list);
724 - int separator_pos = find_separator(tr->text, cl_separators);
724 + ssize_t separator_pos = find_separator(tr->text, cl_separators);
725
726 if (separator_pos == 0) {
727 struct strbuf sb = STRBUF_INIT;
@@ -763,9 +763,9 @@ static const char *next_line(const char *str)
763 /*
764 * Return the position of the start of the last line. If len is 0, return -1.
765 */
766 -static int last_line(const char *buf, size_t len)
766 +static ssize_t last_line(const char *buf, size_t len)
767 {
768 - int i;
768 + ssize_t i;
769 if (len == 0)
770 return -1;
771 if (len == 1)
@@ -788,7 +788,7 @@ static int last_line(const char *buf, size_t len)
788 * Return the position of the start of the patch or the length of str if there
789 * is no patch in the message.
790 */
791 -static int find_patch_start(const char *str)
791 +static size_t find_patch_start(const char *str)
792 {
793 const char *s;
794
@@ -804,10 +804,11 @@ static int find_patch_start(const char *str)
804 * Return the position of the first trailer line or len if there are no
805 * trailers.
806 */
807 -static int find_trailer_start(const char *buf, size_t len)
807 +static size_t find_trailer_start(const char *buf, size_t len)
808 {
809 const char *s;
810 - int end_of_title, l, only_spaces = 1;
810 + ssize_t end_of_title, l;
811 + int only_spaces = 1;
812 int recognized_prefix = 0, trailer_lines = 0, non_trailer_lines = 0;
813 /*
814 * Number of possible continuation lines encountered. This will be
@@ -838,7 +839,7 @@ static int find_trailer_start(const char *buf, size_t len)
839 l = last_line(buf, l)) {
840 const char *bol = buf + l;
841 const char **p;
841 - int separator_pos;
842 + ssize_t separator_pos;
843
844 if (bol[0] == comment_line_char) {
845 non_trailer_lines += possible_continuation_lines;
@@ -899,14 +900,14 @@ continue_outer_loop:
900 }
901
902 /* Return the position of the end of the trailers. */
902 -static int find_trailer_end(const char *buf, size_t len)
903 +static size_t find_trailer_end(const char *buf, size_t len)
904 {
905 return len - ignore_non_trailer(buf, len);
906 }
907
908 static int ends_with_blank_line(const char *buf, size_t len)
909 {
909 - int ll = last_line(buf, len);
910 + ssize_t ll = last_line(buf, len);
911 if (ll < 0)
912 return 0;
913 return is_blank_line(buf + ll);
@@ -939,10 +940,10 @@ static void unfold_value(struct strbuf *val)
940 strbuf_release(&out);
941 }
942
942 -static int process_input_file(FILE *outfile,
943 - const char *str,
944 - struct list_head *head,
945 - const struct process_trailer_options *opts)
943 +static size_t process_input_file(FILE *outfile,
944 + const char *str,
945 + struct list_head *head,
946 + const struct process_trailer_options *opts)
947 {
948 struct trailer_info info;
949 struct strbuf tok = STRBUF_INIT;
@@ -1032,7 +1033,7 @@ void process_trailers(const char *file,
1033 {
1034 LIST_HEAD(head);
1035 struct strbuf sb = STRBUF_INIT;
1035 - int trailer_end;
1036 + size_t trailer_end;
1037 FILE *outfile = stdout;
1038
1039 ensure_configured();
@@ -1132,7 +1133,7 @@ static void format_trailer_info(struct strbuf *out,
1133
1134 for (i = 0; i < info->trailer_nr; i++) {
1135 char *trailer = info->trailers[i];
1135 - int separator_pos = find_separator(trailer, separators);
1136 + ssize_t separator_pos = find_separator(trailer, separators);
1137
1138 if (separator_pos >= 1) {
1139 struct strbuf tok = STRBUF_INIT;