Skip to content

Commit 695d066

Browse files
lweiss-fairphoneandersson
authored andcommitted
storage: fix out of bounds read
Given that shadow_len is size_t (unsigned integer), subtracting a number from it will make it wrap around < 0 and become positive again so the subsequent "if (n > 0)" check will be mostly useless. On AOSP this makes rmtfs segfault, on Linux distributions rmtfs happily reads beyond the end of the buf. Fix this by casting both parameters to ssize_t (which is signed) to correctly use the if and not read beyond the end of shadow_buf. Relevant trace using extra debug statements: storage_populate_shadow_buf: file=/dev/disk/by-partlabel/fsg shadow_buf=0xffffa5217060 shadow_len=0x280000 <snip> storage_pread: memcpy shadow_buf=0xffffa5217060 offset=0x27fc00 n=0x200 storage_pread: memcpy shadow_buf=0xffffa5217060 offset=0x27fe00 n=0x200 storage_pread: memcpy shadow_buf=0xffffa5217060 offset=0x280000 n=0x0 - don't read! storage_pread: memcpy shadow_buf=0xffffa5217060 offset=0x280200 n=0x200 storage_pread: memcpy shadow_buf=0xffffa5217060 offset=0x280400 n=0x200 storage_pread: memcpy shadow_buf=0xffffa5217060 offset=0x280600 n=0x200 storage_pread: memcpy shadow_buf=0xffffa5217060 offset=0x280800 n=0x200 <snip> Signed-off-by: Luca Weiss <[email protected]>
1 parent b08ef6f commit 695d066

File tree

1 file changed

+1
-1
lines changed

1 file changed

+1
-1
lines changed

storage.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -202,7 +202,7 @@ ssize_t storage_pread(const struct rmtfd *rmtfd, void *buf, size_t nbyte, off_t
202202
if (!storage_read_only) {
203203
n = pread(rmtfd->fd, buf, nbyte, offset);
204204
} else {
205-
n = MIN(nbyte, rmtfd->shadow_len - offset);
205+
n = MIN((ssize_t)nbyte, (ssize_t)rmtfd->shadow_len - offset);
206206
if (n > 0)
207207
memcpy(buf, (char*)rmtfd->shadow_buf + offset, n);
208208
else

0 commit comments

Comments
 (0)