POSIX implementation of CopyDirectory has potential TOC-TOU issues |
||||
Issue descriptionCopyDirectory can be found here: https://chromium.googlesource.com/chromium/+/c83733ab23cde8084b9f54c4cab080a6524c1d8b/base/file_util_posix.cc#282 It uses FileEnumerator to get pathnames and stat information for each file. We check the stat returned by the FileEnumerator to see if the file is a regular file, and then we call CopyFile, which actually performs the open. At this point the file could have been swapped with something else (say, a symlink, or a pipe which will force us to block during the open call). A more robust solution would be to open the file first (with O_NONBLOCK, etc.) and then fstat the fd to find out the mode of the exact object we're going to copy.
,
Sep 21 2017
,
Sep 21 2017
ejcaruso: Trying to figure out expectations here. Is this something you want to fix, or you want me to fix?
,
Sep 21 2017
Oh hey, an incoming CL.
,
Sep 28 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/bca21fc58cb2e2275fc00e8eb5700047749f7fce commit bca21fc58cb2e2275fc00e8eb5700047749f7fce Author: Eric Caruso <ejcaruso@chromium.org> Date: Thu Sep 28 00:34:34 2017 file_util_posix: refactor CopyDirectory Rearrange the loop so that we can use continue to go to the next file. Also factor the copying bits of CopyFile so that CopyDirectory can use them with files that are already open, so it can open/fstat the files itself and be resilient to time-of-check/time-of-use attacks (such as the attacker replacing a file that was just stated but not yet opened with a pipe that it will block reading from). Bug: 767138 Test: existing + new unit tests Change-Id: I0c2164b0165850834323d860d1d390ac895737e9 Reviewed-on: https://chromium-review.googlesource.com/677563 Commit-Queue: Eric Caruso <ejcaruso@chromium.org> Reviewed-by: Lei Zhang <thestig@chromium.org> Cr-Commit-Position: refs/heads/master@{#504813} [modify] https://crrev.com/bca21fc58cb2e2275fc00e8eb5700047749f7fce/base/files/file_util.h [modify] https://crrev.com/bca21fc58cb2e2275fc00e8eb5700047749f7fce/base/files/file_util_posix.cc [modify] https://crrev.com/bca21fc58cb2e2275fc00e8eb5700047749f7fce/base/files/file_util_unittest.cc
,
Sep 28 2017
|
||||
►
Sign in to add a comment |
||||
Comment 1 by ejcaruso@chromium.org
, Sep 20 2017