Bug 249533 - fsck_msdosfs: Integer overflow in checksize() for files close to 4GiB in size on 32-bit platform
Summary: fsck_msdosfs: Integer overflow in checksize() for files close to 4GiB in size...
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: bin (show other bugs)
Version: CURRENT
Hardware: Any Any
: --- Affects Some People
Assignee: Xin LI
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-09-22 20:02 UTC by Xin LI
Modified: 2020-09-23 08:49 UTC (History)
1 user (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Xin LI freebsd_committer freebsd_triage 2020-09-22 20:02:24 UTC
Reference: https://android-review.googlesource.com/c/platform/external/fsck_msdos/+/1428461

In checksize(), the physicalSize is represented in size_t, which is 32-bit on 32-bit platforms.  When the file size is within (4GiB-cluster size..4GiB) range, the count of cluster * cluster size would be exactly 4 GiB, which will wrap to 0 and cause the file to be errornously truncated.
Comment 1 Ed Maste freebsd_committer freebsd_triage 2020-09-22 21:09:27 UTC
(In reply to Xin LI from comment #0)
I agree with your comments in the Android review
Comment 2 commit-hook freebsd_committer freebsd_triage 2020-09-23 06:52:56 UTC
A commit references this bug:

Author: delphij
Date: Wed Sep 23 06:52:23 UTC 2020
New revision: 366064
URL: https://svnweb.freebsd.org/changeset/base/366064

Log:
  sbin/fsck_msdosfs: Fix an integer overflow on 32-bit platforms.

  The purpose of checksize() is to verify that the referenced cluster
  chain size matches the recorded file size (up to 2^32 - 1) in the
  directory entry. We follow the cluster chain, then multiple the
  cluster count by bytes per cluster to get the physical size, then
  check it against the recorded size.

  When a file is close to 4 GiB (between 4GiB - cluster size and 4GiB,
  both non-inclusive), the product of cluster count and bytes per
  cluster would be exactly 4 GiB. On 32-bit systems, because size_t
  is 32-bit, this would wrap back to 0, which will cause the file be
  truncated to 0.

  Fix this by using 64-bit physicalSize instead.

  This fix is inspired by an Android change request at
  https://android-review.googlesource.com/c/platform/external/fsck_msdos/+/1428461

  PR:		249533
  Reviewed by:	kevlo
  MFC after:	3 days
  Differential Revision:	https://reviews.freebsd.org/D26524

Changes:
  head/sbin/fsck_msdosfs/dir.c