Skip to content

Fix TOC/TOU in directory::copy#202

Merged
bugdea1er merged 1 commit into
mainfrom
directory-copy
Apr 23, 2025
Merged

Fix TOC/TOU in directory::copy#202
bugdea1er merged 1 commit into
mainfrom
directory-copy

Conversation

@bugdea1er
Copy link
Copy Markdown
Owner

Underlying implementation of directory::copy now opens a directory_iterator on the from path to avoid TOC/TOU vulnerability

@bugdea1er bugdea1er requested a review from Copilot April 22, 2025 20:08
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR aims to fix a TOC/TOU vulnerability in the implementation of directory::copy by opening a directory_iterator on the source path and copying its contents individually. Key changes include:

  • Introducing a new helper function, copy_directory, to iterate over and copy directory contents safely.
  • Updating the directory::copy function to use copy_directory.
  • Adjusting .clang-tidy configuration to ignore naming length warnings for the new iterator variable "it".

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
src/directory.cpp Added copy_directory function and updated directory::copy implementation.
.clang-tidy Updated ignored identifiers list to include the new variable name "it".
Comments suppressed due to low confidence (2)

src/directory.cpp:25

  • [nitpick] Consider using a more descriptive variable name instead of 'it' (e.g. 'dirIter') to improve readability.
fs::directory_iterator it = fs::directory_iterator(from, ec);

src/directory.cpp:30

  • [nitpick] Using fs::directory_iterator() as the end iterator can be less clear; consider storing the end iterator in a variable to clarify the loop condition.
for (; !ec && it != fs::directory_iterator(); it.increment(ec)) {

@bugdea1er bugdea1er requested a review from Copilot April 22, 2025 20:20
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR addresses a potential TOC/TOU vulnerability in directory copying by replacing direct calls to fs::copy with a new copy_directory function that uses fs::directory_iterator. Key changes include:

  • Introducing copy_directory to open a directory iterator on the source path.
  • Updating move and directory::copy functions to use copy_directory.
  • Adjusting .clang-tidy options to ignore identifier-length warnings for the variable "it".

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
src/directory.cpp Introduces copy_directory and replaces fs::copy calls with this function.
.clang-tidy Updates settings to ignore identifier-length for parameters "it".
Comments suppressed due to low confidence (1)

src/directory.cpp:21

  • Consider ensuring that the target directory 'to' exists before copying files. This would avoid potential runtime errors when constructing file paths inside the copy loop.
fs::directory_iterator it = fs::directory_iterator(from, ec);

@bugdea1er bugdea1er marked this pull request as ready for review April 22, 2025 20:30
@bugdea1er bugdea1er requested a review from Copilot April 22, 2025 20:36
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR improves the security of directory copying by mitigating TOC/TOU vulnerabilities and refactoring the directory deletion function.

  • Renames the recursive deletion function to remove_directory for clarity.
  • Iterates through directory entries in directory::copy to avoid time-of-check to time-of-use issues.
  • Updates .clang-tidy configuration to ignore the "it" parameter for identifier-length checks.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
src/directory.cpp Refactored deletion function and replaced fs::copy with iteration to mitigate TOC/TOU vulnerability.
.clang-tidy Adjusted ignored names for readability-identifier-length checks.

Repository owner deleted a comment from Copilot AI Apr 22, 2025
@bugdea1er bugdea1er merged commit 85e1af4 into main Apr 23, 2025
16 checks passed
@bugdea1er bugdea1er deleted the directory-copy branch April 23, 2025 06:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants