I found that the validation contained an off-by-one error. The expression '(u32)(usa_ofs + (usa_count * 2)) > size' used 'usa_count' after it had been decremented to skip the update sequence number entry. Consequently, the code could read out of bounds, up to two bytes past the end of the MST-protected record.
Furthermore, as documented in the comment in layout.h for "NTFS_RECORD" and also on MSDN for "MULTI_SECTOR_HEADER", the update sequence array must end before the last le16 in the first sector --- not merely before the end of the record. Fix the validation and move it into a helper function, as it was done identically in the read and write paths. Signed-off-by: Eric Biggers <ebigge...@gmail.com> --- libntfs-3g/mst.c | 45 +++++++++++++++++++++++++++------------------ 1 file changed, 27 insertions(+), 18 deletions(-) diff --git a/libntfs-3g/mst.c b/libntfs-3g/mst.c index 9dff773..88b9cdb 100644 --- a/libntfs-3g/mst.c +++ b/libntfs-3g/mst.c @@ -31,6 +31,21 @@ #include "mst.h" #include "logging.h" +/* + * Basic validation of a NTFS multi-sector record. The record size must be a + * multiple of the sector size; and the update sequence array must be properly + * aligned, of the expected length, and must end before the last le16 in the + * first sector. + */ +static BOOL +is_valid_record(u32 size, u16 usa_ofs, u16 usa_count) +{ + return size % NTFS_BLOCK_SIZE == 0 && + usa_ofs % 2 == 0 && + usa_count == 1 + (size / NTFS_BLOCK_SIZE) && + usa_ofs + ((u32)usa_count * 2) <= NTFS_BLOCK_SIZE - 2; +} + /** * ntfs_mst_post_read_fixup - deprotect multi sector transfer protected data * @b: pointer to the data to deprotect @@ -57,12 +72,9 @@ int ntfs_mst_post_read_fixup_warn(NTFS_RECORD *b, const u32 size, /* Setup the variables. */ usa_ofs = le16_to_cpu(b->usa_ofs); - /* Decrement usa_count to get number of fixups. */ - usa_count = le16_to_cpu(b->usa_count) - 1; - /* Size and alignment checks. */ - if (size & (NTFS_BLOCK_SIZE - 1) || usa_ofs & 1 || - (u32)(usa_ofs + (usa_count * 2)) > size || - (size >> NTFS_BLOCK_SIZE_BITS) != usa_count) { + usa_count = le16_to_cpu(b->usa_count); + + if (!is_valid_record(size, usa_ofs, usa_count)) { errno = EINVAL; if (warn) { ntfs_log_perror("%s: magic: 0x%08lx size: %ld " @@ -91,7 +103,7 @@ int ntfs_mst_post_read_fixup_warn(NTFS_RECORD *b, const u32 size, /* * Check for incomplete multi sector transfer(s). */ - while (usa_count--) { + while (--usa_count) { if (*data_pos != usn) { /* * Incomplete multi sector transfer detected! )-: @@ -109,10 +121,10 @@ int ntfs_mst_post_read_fixup_warn(NTFS_RECORD *b, const u32 size, data_pos += NTFS_BLOCK_SIZE/sizeof(u16); } /* Re-setup the variables. */ - usa_count = le16_to_cpu(b->usa_count) - 1; + usa_count = le16_to_cpu(b->usa_count); data_pos = (u16*)b + NTFS_BLOCK_SIZE/sizeof(u16) - 1; /* Fixup all sectors. */ - while (usa_count--) { + while (--usa_count) { /* * Increment position in usa and restore original data from * the usa into the data buffer. @@ -171,12 +183,9 @@ int ntfs_mst_pre_write_fixup(NTFS_RECORD *b, const u32 size) } /* Setup the variables. */ usa_ofs = le16_to_cpu(b->usa_ofs); - /* Decrement usa_count to get number of fixups. */ - usa_count = le16_to_cpu(b->usa_count) - 1; - /* Size and alignment checks. */ - if (size & (NTFS_BLOCK_SIZE - 1) || usa_ofs & 1 || - (u32)(usa_ofs + (usa_count * 2)) > size || - (size >> NTFS_BLOCK_SIZE_BITS) != usa_count) { + usa_count = le16_to_cpu(b->usa_count); + + if (!is_valid_record(size, usa_ofs, usa_count)) { errno = EINVAL; ntfs_log_perror("%s", __FUNCTION__); return -1; @@ -195,7 +204,7 @@ int ntfs_mst_pre_write_fixup(NTFS_RECORD *b, const u32 size) /* Position in data of first le16 that needs fixing up. */ data_pos = (le16*)b + NTFS_BLOCK_SIZE/sizeof(le16) - 1; /* Fixup all sectors. */ - while (usa_count--) { + while (--usa_count) { /* * Increment the position in the usa and save the * original data from the data buffer into the usa. @@ -223,7 +232,7 @@ void ntfs_mst_post_write_fixup(NTFS_RECORD *b) u16 *usa_pos, *data_pos; u16 usa_ofs = le16_to_cpu(b->usa_ofs); - u16 usa_count = le16_to_cpu(b->usa_count) - 1; + u16 usa_count = le16_to_cpu(b->usa_count); ntfs_log_trace("Entering\n"); @@ -234,7 +243,7 @@ void ntfs_mst_post_write_fixup(NTFS_RECORD *b) data_pos = (u16*)b + NTFS_BLOCK_SIZE/sizeof(u16) - 1; /* Fixup all sectors. */ - while (usa_count--) { + while (--usa_count) { /* * Increment position in usa and restore original data from * the usa into the data buffer. -- 2.9.0 ------------------------------------------------------------------------------ What NetFlow Analyzer can do for you? Monitors network bandwidth and traffic patterns at an interface-level. Reveals which users, apps, and protocols are consuming the most bandwidth. Provides multi-vendor support for NetFlow, J-Flow, sFlow and other flows. Make informed decisions using capacity planning reports.http://sdm.link/zohodev2dev _______________________________________________ ntfs-3g-devel mailing list ntfs-3g-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/ntfs-3g-devel