Prevent buffer overflows when path is too long

Some buffers created with PATH_MAX length are not checked when being written, and can overflow if PATH_MAX is not big enough to hold the path. Replace those buffers by strbufs so that their size is automatically grown if necessary. They are created as static local variables to avoid reallocating memory on each call. Note that prefix_filename() returns this static buffer so each callers should copy or use the string immediately (this is currently true). Reported-by: Wataru Noguchi <wnoguchi.0727@gmail.com> Signed-off-by: Antoine Pelisse <apelisse@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Antoine Pelisse committed Dec 14, 2013 at 12:31 UTC fc2b6214542a46f97d7067b2f7df530ed37737a7
3 files changed +42 -36
abspath.c
+9 -7
@@ -215,23 +215,25 @@ const char *absolute_path(const char *path)
215 */
216 const char *prefix_filename(const char *pfx, int pfx_len, const char *arg)
217 {
218 - static char path[PATH_MAX];
218 + static struct strbuf path = STRBUF_INIT;
219 #ifndef GIT_WINDOWS_NATIVE
220 if (!pfx_len || is_absolute_path(arg))
221 return arg;
222 - memcpy(path, pfx, pfx_len);
223 - strcpy(path + pfx_len, arg);
222 + strbuf_reset(&path);
223 + strbuf_add(&path, pfx, pfx_len);
224 + strbuf_addstr(&path, arg);
225 #else
226 char *p;
227 /* don't add prefix to absolute paths, but still replace '\' by '/' */
228 + strbuf_reset(&path);
229 if (is_absolute_path(arg))
230 pfx_len = 0;
231 else if (pfx_len)
230 - memcpy(path, pfx, pfx_len);
231 - strcpy(path + pfx_len, arg);
232 - for (p = path + pfx_len; *p; p++)
232 + strbuf_add(&path, pfx, pfx_len);
233 + strbuf_addstr(&path, arg);
234 + for (p = path.buf + pfx_len; *p; p++)
235 if (*p == '\\')
236 *p = '/';
237 #endif
236 - return path;
238 + return path.buf;
239 }
diffcore-order.c
+6 -5
@@ -73,15 +73,16 @@ struct pair_order {
73 static int match_order(const char *path)
74 {
75 int i;
76 - char p[PATH_MAX];
76 + static struct strbuf p = STRBUF_INIT;
77
78 for (i = 0; i < order_cnt; i++) {
79 - strcpy(p, path);
80 - while (p[0]) {
79 + strbuf_reset(&p);
80 + strbuf_addstr(&p, path);
81 + while (p.buf[0]) {
82 char *cp;
82 - if (!fnmatch(order[i], p, 0))
83 + if (!fnmatch(order[i], p.buf, 0))
84 return i;
84 - cp = strrchr(p, '/');
85 + cp = strrchr(p.buf, '/');
86 if (!cp)
87 break;
88 *cp = 0;
unpack-trees.c
+27 -24
@@ -830,23 +830,24 @@ static int unpack_callback(int n, unsigned long mask, unsigned long dirmask, str
830 }
831
832 static int clear_ce_flags_1(struct cache_entry **cache, int nr,
833 - char *prefix, int prefix_len,
833 + struct strbuf *prefix,
834 int select_mask, int clear_mask,
835 struct exclude_list *el, int defval);
836
837 /* Whole directory matching */
838 static int clear_ce_flags_dir(struct cache_entry **cache, int nr,
839 - char *prefix, int prefix_len,
839 + struct strbuf *prefix,
840 char *basename,
841 int select_mask, int clear_mask,
842 struct exclude_list *el, int defval)
843 {
844 struct cache_entry **cache_end;
845 int dtype = DT_DIR;
846 - int ret = is_excluded_from_list(prefix, prefix_len,
846 + int ret = is_excluded_from_list(prefix->buf, prefix->len,
847 basename, &dtype, el);
848 + int rc;
849
849 - prefix[prefix_len++] = '/';
850 + strbuf_addch(prefix, '/');
851
852 /* If undecided, use matching result of parent dir in defval */
853 if (ret < 0)
@@ -854,7 +855,7 @@ static int clear_ce_flags_dir(struct cache_entry **cache, int nr,
855
856 for (cache_end = cache; cache_end != cache + nr; cache_end++) {
857 struct cache_entry *ce = *cache_end;
857 - if (strncmp(ce->name, prefix, prefix_len))
858 + if (strncmp(ce->name, prefix->buf, prefix->len))
859 break;
860 }
861
@@ -865,10 +866,12 @@ static int clear_ce_flags_dir(struct cache_entry **cache, int nr,
866 * calling clear_ce_flags_1(). That function will call
867 * the expensive is_excluded_from_list() on every entry.
868 */
868 - return clear_ce_flags_1(cache, cache_end - cache,
869 - prefix, prefix_len,
870 - select_mask, clear_mask,
871 - el, ret);
869 + rc = clear_ce_flags_1(cache, cache_end - cache,
870 + prefix,
871 + select_mask, clear_mask,
872 + el, ret);
873 + strbuf_setlen(prefix, prefix->len - 1);
874 + return rc;
875 }
876
877 /*
@@ -887,7 +890,7 @@ static int clear_ce_flags_dir(struct cache_entry **cache, int nr,
890 * Top level path has prefix_len zero.
891 */
892 static int clear_ce_flags_1(struct cache_entry **cache, int nr,
890 - char *prefix, int prefix_len,
893 + struct strbuf *prefix,
894 int select_mask, int clear_mask,
895 struct exclude_list *el, int defval)
896 {
@@ -907,10 +910,10 @@ static int clear_ce_flags_1(struct cache_entry **cache, int nr,
910 continue;
911 }
912
910 - if (prefix_len && strncmp(ce->name, prefix, prefix_len))
913 + if (prefix->len && strncmp(ce->name, prefix->buf, prefix->len))
914 break;
915
913 - name = ce->name + prefix_len;
916 + name = ce->name + prefix->len;
917 slash = strchr(name, '/');
918
919 /* If it's a directory, try whole directory match first */
@@ -918,29 +921,26 @@ static int clear_ce_flags_1(struct cache_entry **cache, int nr,
921 int processed;
922
923 len = slash - name;
921 - memcpy(prefix + prefix_len, name, len);
924 + strbuf_add(prefix, name, len);
925
923 - /*
924 - * terminate the string (no trailing slash),
925 - * clear_c_f_dir needs it
926 - */
927 - prefix[prefix_len + len] = '\0';
926 processed = clear_ce_flags_dir(cache, cache_end - cache,
929 - prefix, prefix_len + len,
930 - prefix + prefix_len,
927 + prefix,
928 + prefix->buf + prefix->len - len,
929 select_mask, clear_mask,
930 el, defval);
931
932 /* clear_c_f_dir eats a whole dir already? */
933 if (processed) {
934 cache += processed;
935 + strbuf_setlen(prefix, prefix->len - len);
936 continue;
937 }
938
940 - prefix[prefix_len + len++] = '/';
939 + strbuf_addch(prefix, '/');
940 cache += clear_ce_flags_1(cache, cache_end - cache,
942 - prefix, prefix_len + len,
941 + prefix,
942 select_mask, clear_mask, el, defval);
943 + strbuf_setlen(prefix, prefix->len - len - 1);
944 continue;
945 }
946
@@ -961,9 +961,12 @@ static int clear_ce_flags(struct cache_entry **cache, int nr,
961 int select_mask, int clear_mask,
962 struct exclude_list *el)
963 {
964 - char prefix[PATH_MAX];
964 + static struct strbuf prefix = STRBUF_INIT;
965 +
966 + strbuf_reset(&prefix);
967 +
968 return clear_ce_flags_1(cache, nr,
966 - prefix, 0,
969 + &prefix,
970 select_mask, clear_mask,
971 el, 0);
972 }