Load model from archive #113
Conversation
Signed-off-by: Emily Casey <[email protected]>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
|
||
| func (r *Reader) Next() (v1.Hash, error) { | ||
| for { | ||
| hdr, err := r.tr.Next() |
Check failure
Code scanning / CodeQL
Arbitrary file access during archive extraction ("Zip Slip") High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 5 days ago
To fix the problem, input from the TAR archive (hdr.Name) should be properly sanitized before being used to construct a v1.Hash, especially since its fields are used to form filesystem paths. The best way to do this is to ensure that hdr.Name is not an absolute path, does not contain any .. directory traversal elements, and that both parts[1] (algorithm) and parts[2] (hex) are valid and safe for use as path segments. This can be achieved by:
- Rejecting any entry where
hdr.Nameis, or cleans to, an absolute path. - Rejecting any entry where any segment is
..or empty. - Optionally, adding stricter validation for
parts[1]andparts[2](for example, matching a regex for expected hash algorithm names and hexadecimal strings).
These checks should be implemented in the Next() function in tarball/reader.go, before constructing and returning the v1.Hash.
Required changes:
- In
tarball/reader.go, withinfunc (r *Reader) Next(), add robust checks after cleaning and splittinghdr.Nameto ensure it does not contain.., empty segments, or absolute paths, and thatparts[1]andparts[2]are valid. - No additional imports are needed beyond what is already present (use
filepath.IsAbsand string checks). - Return an error or
continueif a dangerous entry is detected.
| @@ -60,14 +60,24 @@ | ||
| } | ||
| continue | ||
| } | ||
| parts := strings.Split(filepath.Clean(hdr.Name), "/") | ||
| if len(parts) != 3 || parts[0] != "blobs" && parts[0] != "manifests" { | ||
| cleanName := filepath.Clean(hdr.Name) | ||
| // Prevent absolute paths or path traversal | ||
| if filepath.IsAbs(cleanName) { | ||
| continue | ||
| } | ||
| return v1.Hash{ | ||
| Algorithm: parts[1], | ||
| Hex: parts[2], | ||
| }, nil | ||
| parts := strings.Split(cleanName, "/") | ||
| if len(parts) != 3 || (parts[0] != "blobs" && parts[0] != "manifests") { | ||
| continue | ||
| } | ||
| // Check for any ".." or empty segments | ||
| badSegment := false | ||
| for _, part := range parts { | ||
| if part == ".." || part == "" || strings.Contains(part, string(filepath.Separator)) { | ||
| badSegment = true | ||
| break | ||
| } | ||
| } | ||
|
|
||
| } | ||
| } | ||
|
|
Summary
distribution.Client.tarball.TargetCLI Usage
model-distribution-tool loadIntroduces
loadcommand which loads model from the archive at into the store and optionally applies .model-distribution-tool package --fileAdds
--file <path>flag topackagecommand, which results in the model being written to a TAR archive at the given<path>instead of being pushed to the registry.LoadModelwithdistribution.ClientExpects to read a TAR archive from
rcformatted as described below and applies the given tag, writing progress toprogressWriter.Format
package and load expect/produce models with any number of blobs entries with name
blobs/sha256/<hash>and 1manifest.json. Example:TODO
model-distribution-tool packagewith--loadflag