@matheusbsbs / matheus-bagliano / commits / 9d2e330b17

ewah_read_mmap: bounds-check mmap reads

The on-disk ewah format tells us how big the ewah data is, and we blindly read that much from the buffer without considering whether the mmap'd data is long enough, which can lead to out-of-bound reads. Let's make sure we have data available before reading it, both for the ewah header/footer as well as for the bit data itself. In particular: - keep our ptr/len pair in sync as we move through the buffer, and check it before each read - check the size for integer overflow (this should be impossible on 64-bit, as the size is given as a 32-bit count of 8-byte words, but is possible on a 32-bit system) - return the number of bytes read as an ssize_t instead of an int, again to prevent integer overflow - compute the return value using a pointer difference; this should yield the same result as the existing code, but makes it more obvious that we got our computations right The included test is far from comprehensive, as it just picks a static point at which to truncate the generated bitmap. But in practice this will hit in the middle of an ewah and make sure we're at least exercising this code. Reported-by: Luat Nguyen <root@l4w.io> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jeff King committed Jun 14, 2018 at 23:31 UTC 9d2e330b1795222c2c816afa46138e7ff4ebec8e

3 files changed +35 -5

ewah/ewah_io.c

+21 -4

@@ -122,16 +122,23 @@ int ewah_serialize_strbuf(struct ewah_bitmap *self, struct strbuf *sb)

122 return ewah_serialize_to(self, write_strbuf, sb);

123 }

124

125 -int ewah_read_mmap(struct ewah_bitmap *self, const void *map, size_t len)

125 +ssize_t ewah_read_mmap(struct ewah_bitmap *self, const void *map, size_t len)

126 {

127 const uint8_t *ptr = map;

128 + size_t data_len;

129 size_t i;

130

131 + if (len < sizeof(uint32_t))

132 + return error("corrupt ewah bitmap: eof before bit size");

133 self->bit_size = get_be32(ptr);

134 ptr += sizeof(uint32_t);

135 + len -= sizeof(uint32_t);

136

137 + if (len < sizeof(uint32_t))

138 + return error("corrupt ewah bitmap: eof before length");

139 self->buffer_size = self->alloc_size = get_be32(ptr);

140 ptr += sizeof(uint32_t);

141 + len -= sizeof(uint32_t);

142

143 REALLOC_ARRAY(self->buffer, self->alloc_size);

144

@@ -141,15 +148,25 @@ int ewah_read_mmap(struct ewah_bitmap *self, const void *map, size_t len)

148 * the endianness conversion in a separate pass to ensure

149 * we're loading 8-byte aligned words.

150 */

144 - memcpy(self->buffer, ptr, self->buffer_size * sizeof(eword_t));

145 - ptr += self->buffer_size * sizeof(eword_t);

151 + data_len = st_mult(self->buffer_size, sizeof(eword_t));

152 + if (len < data_len)

153 + return error("corrupt ewah bitmap: eof in data "

154 + "(%"PRIuMAX" bytes short)",

155 + (uintmax_t)(data_len - len));

156 + memcpy(self->buffer, ptr, data_len);

157 + ptr += data_len;

158 + len -= data_len;

159

160 for (i = 0; i < self->buffer_size; ++i)

161 self->buffer[i] = ntohll(self->buffer[i]);

162

163 + if (len < sizeof(uint32_t))

164 + return error("corrupt ewah bitmap: eof before rlw");

165 self->rlw = self->buffer + get_be32(ptr);

166 + ptr += sizeof(uint32_t);

167 + len -= sizeof(uint32_t);

168

152 - return (3 * 4) + (self->buffer_size * 8);

169 + return ptr - (const uint8_t *)map;

170 }

171

172 int ewah_deserialize(struct ewah_bitmap *self, int fd)

ewah/ewok.h

+1 -1

@@ -91,7 +91,7 @@ int ewah_serialize_native(struct ewah_bitmap *self, int fd);

91 int ewah_serialize_strbuf(struct ewah_bitmap *self, struct strbuf *);

93 int ewah_deserialize(struct ewah_bitmap *self, int fd);

94 -int ewah_read_mmap(struct ewah_bitmap *self, const void *map, size_t len);

94 +ssize_t ewah_read_mmap(struct ewah_bitmap *self, const void *map, size_t len);

96 uint32_t ewah_checksum(struct ewah_bitmap *self);

t/t5310-pack-bitmaps.sh

+13

@@ -331,4 +331,17 @@ test_expect_success 'pack reuse respects --incremental' '

331 git show-index <empty.idx >actual &&

332 test_cmp expect actual

333 '

334 +

335 +test_expect_success 'truncated bitmap fails gracefully' '

336 + git repack -ad &&

337 + git rev-list --use-bitmap-index --count --all >expect &&

338 + bitmap=$(ls .git/objects/pack/*.bitmap) &&

339 + test_when_finished "rm -f $bitmap" &&

340 + head -c 512 <$bitmap >$bitmap.tmp &&

341 + mv -f $bitmap.tmp $bitmap &&

342 + git rev-list --use-bitmap-index --count --all >actual 2>stderr &&

343 + test_cmp expect actual &&

344 + test_i18ngrep corrupt stderr

345 +'

346 +

347 test_done