@matheusbsbs / matheus-bagliano / commits / 7fec10b7f4
Fork 0

index: be careful when handling long names

We currently use lower 12-bit (masked with CE_NAMEMASK) in the ce_flags field to store the length of the name in cache_entry, without checking the length parameter given to create_ce_flags(). This can make us store incorrect length. Currently we are mostly protected by the fact that many codepaths first copy the path in a variable of size PATH_MAX, which typically is 4096 that happens to match the limit, but that feels like a bug waiting to happen. Besides, that would not allow us to shorten the width of CE_NAMEMASK to use the bits for new flags. This redefines the meaning of the name length stored in the cache_entry. A name that does not fit is represented by storing CE_NAMEMASK in the field, and the actual length needs to be computed by actually counting the bytes in the name[] field. This way, only the unusually long paths need to suffer. Signed-off-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

Junio C Hamano committed Jan 18, 2008 at 23:42 UTC 7fec10b7f41fa32e71aa6377bd04cd7c6fb419e0
3 files changed +46 -3
cache.h
+15 -2
@@ -131,8 +131,21 @@ struct cache_entry {
131 #define CE_UPDATE (0x10000)
132 #define CE_REMOVE (0x20000)
133
134 -#define create_ce_flags(len, stage) ((len) | ((stage) << CE_STAGESHIFT))
135 -#define ce_namelen(ce) (CE_NAMEMASK & (ce)->ce_flags)
134 +static inline unsigned create_ce_flags(size_t len, unsigned stage)
135 +{
136 + if (len >= CE_NAMEMASK)
137 + len = CE_NAMEMASK;
138 + return (len | (stage << CE_STAGESHIFT));
139 +}
140 +
141 +static inline size_t ce_namelen(const struct cache_entry *ce)
142 +{
143 + size_t len = ce->ce_flags & CE_NAMEMASK;
144 + if (len < CE_NAMEMASK)
145 + return len;
146 + return strlen(ce->name + CE_NAMEMASK) + CE_NAMEMASK;
147 +}
148 +
149 #define ce_size(ce) cache_entry_size(ce_namelen(ce))
150 #define ondisk_ce_size(ce) ondisk_cache_entry_size(ce_namelen(ce))
151 #define ce_stage(ce) ((CE_STAGEMASK & (ce)->ce_flags) >> CE_STAGESHIFT)
read-cache.c
+11 -1
@@ -928,6 +928,8 @@ int read_index(struct index_state *istate)
928
929 static void convert_from_disk(struct ondisk_cache_entry *ondisk, struct cache_entry *ce)
930 {
931 + size_t len;
932 +
933 ce->ce_ctime = ntohl(ondisk->ctime.sec);
934 ce->ce_mtime = ntohl(ondisk->mtime.sec);
935 ce->ce_dev = ntohl(ondisk->dev);
@@ -939,7 +941,15 @@ static void convert_from_disk(struct ondisk_cache_entry *ondisk, struct cache_en
941 /* On-disk flags are just 16 bits */
942 ce->ce_flags = ntohs(ondisk->flags);
943 hashcpy(ce->sha1, ondisk->sha1);
942 - memcpy(ce->name, ondisk->name, ce_namelen(ce)+1);
944 +
945 + len = ce->ce_flags & CE_NAMEMASK;
946 + if (len == CE_NAMEMASK)
947 + len = strlen(ondisk->name);
948 + /*
949 + * NEEDSWORK: If the original index is crafted, this copy could
950 + * go unchecked.
951 + */
952 + memcpy(ce->name, ondisk->name, len + 1);
953 }
954
955 /* remember to discard_cache() before reading a different cache! */
t/t0000-basic.sh
+20
@@ -297,4 +297,24 @@ test_expect_success 'absolute path works as expected' '
297 test "$sym" = "$(test-absolute-path $dir2/syml)"
298 '
299
300 +test_expect_success 'very long name in the index handled sanely' '
301 +
302 + a=a && # 1
303 + a=$a$a$a$a$a$a$a$a$a$a$a$a$a$a$a$a && # 16
304 + a=$a$a$a$a$a$a$a$a$a$a$a$a$a$a$a$a && # 256
305 + a=$a$a$a$a$a$a$a$a$a$a$a$a$a$a$a$a && # 4096
306 + a=${a}q &&
307 +
308 + >path4 &&
309 + git update-index --add path4 &&
310 + (
311 + git ls-files -s path4 |
312 + sed -e "s/ .*/ /" |
313 + tr -d "\012"
314 + echo "$a"
315 + ) | git update-index --index-info &&
316 + len=$(git ls-files "a*" | wc -c) &&
317 + test $len = 4098
318 +'
319 +
320 test_done