LFS Upload SHA256 Validation

[Yiling-J]

Security Issue:

The LFS file upload API (/info/lfs/:oid/:size) does not validate whether the file's SHA256 matches the LFS object's OID.

GitLab Handling Method:

GitLab’s LFS file upload process involves interactions between Rails and Go services, and the logic is quite complex. However, the following two PRs/issues provide insight into how they handle this:

• Git LFS: Validate upload size and SHA256 in Rails Merge Request 85819

• Object Store uses undocumented tmp/ for uploads Issue 416222

Their upload logic is as follows: First, the file is uploaded to a temporary directory. Once the SHA256 validation is successful, the file is copied to the actual directory.

Fix:

Adopting the same approach as GitLab:

• All LFS files, regardless of size, will be uploaded using an intermediate mode instead of using pre-signed URLs.
• The file is first uploaded to the lfs/tmp/{oid_key} directory while performing SHA256 validation.
• If the validation succeeds, the file is moved from lfs/tmp/{oid_key} to lfs/{oid_key} and the database is updated.
• Regardless of success or failure, the temporary file in lfs/tmp/{oid_key} will be deleted.

PR Details:

• The minio client has been updated to add the UploadAndValidate method, which uploads the file while performing the validation.
• The UploadAndValidate method includes part size and concurrency restrictions to prevent excessive memory usage. The maximum memory used for a single upload is approximately 25MB (5 * 5). The default git lfs concurrency is 8, which means if you upload more than 8 files, the estimated memory used will be 8 * 25 = 200MB.
• The LfsUpload method in the GitHTTPComponent has been updated to use UploadAndValidate.
• The logic in the LfsUpload method for checking file existence has been adjusted. Previously, any error from StatObject was considered as the file not existing. Now, only specific errors are checked.
• The buildUploadLink method in the GitHTTPComponent no longer checks file size and always returns the server's intermediate upload URL.

[ganisback]

LGTM

[starship-github]

The StarShip CodeReviewer was triggered but terminated because it encountered an issue: The MR state is not opened.

Tips

CodeReview Commands (invoked as MR or PR comments)

• @codegpt /review to trigger an code review.
• @codegpt /evaluate to trigger code evaluation process.
• @codegpt /describe to regenerate the summary of the MR.
• @codegpt /secscan to scan security vulnerabilities for the MR or the Repository.
• @codegpt /help to get help.

CodeReview Discussion Chat

There are 2 ways to chat with Starship CodeReview:

• Review comments: Directly reply to a review comment made by StarShip.
  Example:
  • @codegpt How to fix this bug?
• Files and specific lines of code (under the "Files changed" tab):
  Tag @codegpt in a new review comment at the desired location with your query.
  Examples:
  • @codegpt generate unit testing code for this code snippet.

Note: Be mindful of the bot's finite context window.
It's strongly recommended to break down tasks such as reading entire modules into smaller chunks.
For a focused discussion, use review comments to chat about specific files and their changes, instead of using the MR/PR comments.

CodeReview Documentation and Community

• Visit our Documentation
  for detailed information on how to use Starship CodeReview.