Skip to content

Commit 2444419

Browse files
mleyvajr100copybara-github
authored andcommitted
Use os.Root API in layer scanning unpacking code to harden against path traversal attacks.
PiperOrigin-RevId: 751441217
1 parent 273eeea commit 2444419

File tree

3 files changed

+200
-76
lines changed

3 files changed

+200
-76
lines changed

artifact/image/layerscanning/image/image.go

Lines changed: 36 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,9 @@ const (
4343
// DefaultMaxFileBytes is the default maximum size of files that will be unpacked. Larger files are ignored.
4444
// The max is large because some files are hundreds of megabytes.
4545
DefaultMaxFileBytes = 1024 * 1024 * 1024 // 1GB
46-
// DefaultMaxSymlinkDepth is the default maximum symlink depth.
46+
// DefaultMaxSymlinkDepth is the default maximum symlink depth. This should be no more than 8,
47+
// since that is the maximum number of symlinks the os.Root API will handle. From the os.Root API,
48+
// "8 is __POSIX_SYMLOOP_MAX (the minimum allowed value for SYMLOOP_MAX), and a common limit".
4749
DefaultMaxSymlinkDepth = 6
4850
)
4951

@@ -103,6 +105,7 @@ type Image struct {
103105
chainLayers []*chainLayer
104106
config *Config
105107
size int64
108+
root *os.Root
106109
ExtractDir string
107110
BaseImageIndex int
108111
}
@@ -186,6 +189,16 @@ func FromV1Image(v1Image v1.Image, config *Config) (*Image, error) {
186189
return nil, fmt.Errorf("failed to create temporary directory: %w", err)
187190
}
188191

192+
// OpenRoot assumes that the provided directory is trusted. In this case, we created the
193+
// imageExtractionPath directory, so it is indeed trusted.
194+
root, err := os.OpenRoot(imageExtractionPath)
195+
if err != nil {
196+
return nil, fmt.Errorf("failed to open root directory: %w", err)
197+
}
198+
// Close the root directory at the end of the function, since no more files will be unpacked
199+
// afterward.
200+
defer root.Close()
201+
189202
baseImageIndex, err := findBaseImageIndex(history)
190203
if err != nil {
191204
baseImageIndex = -1
@@ -194,14 +207,15 @@ func FromV1Image(v1Image v1.Image, config *Config) (*Image, error) {
194207
outputImage := &Image{
195208
chainLayers: chainLayers,
196209
config: config,
210+
root: root,
197211
ExtractDir: imageExtractionPath,
198212
BaseImageIndex: baseImageIndex,
199213
}
200214

201215
// Add the root directory to each chain layer. If this is not done, then the virtual paths won't
202216
// be rooted, and traversal in the virtual filesystem will be broken.
203217
if err := addRootDirectoryToChainLayers(outputImage.chainLayers, imageExtractionPath); err != nil {
204-
return handleImageError(outputImage, fmt.Errorf("failed to add root directory to chain layers: %w", err))
218+
return nil, handleImageError(outputImage, fmt.Errorf("failed to add root directory to chain layers: %w", err))
205219
}
206220

207221
// Since the layers are in reverse order, the v1LayerIndex starts at the last layer and works
@@ -221,22 +235,20 @@ func FromV1Image(v1Image v1.Image, config *Config) (*Image, error) {
221235
layerDir := layerDirectory(i)
222236

223237
// Create the chain layer directory if it doesn't exist.
224-
// Use filepath here as it is a path that will be written to disk.
225-
dirPath := filepath.Join(imageExtractionPath, layerDir)
226-
if err := os.Mkdir(dirPath, dirPermission); err != nil && !errors.Is(err, fs.ErrExist) {
227-
return handleImageError(outputImage, fmt.Errorf("failed to create chain layer directory: %w", err))
238+
if err := root.Mkdir(layerDir, dirPermission); err != nil && !errors.Is(err, fs.ErrExist) {
239+
return nil, handleImageError(outputImage, fmt.Errorf("failed to create chain layer directory: %w", err))
228240
}
229241

230242
if v1LayerIndex < 0 {
231-
return handleImageError(outputImage, fmt.Errorf("mismatch between v1 layers and chain layers, on v1 layer index %d, but only %d v1 layers", v1LayerIndex, len(v1Layers)))
243+
return nil, handleImageError(outputImage, fmt.Errorf("mismatch between v1 layers and chain layers, on v1 layer index %d, but only %d v1 layers", v1LayerIndex, len(v1Layers)))
232244
}
233245

234246
chainLayersToFill := chainLayers[i:]
235247

236248
v1Layer := v1Layers[v1LayerIndex]
237249
layerReader, err := v1Layer.Uncompressed()
238250
if err != nil {
239-
return handleImageError(outputImage, err)
251+
return nil, handleImageError(outputImage, err)
240252
}
241253
v1LayerIndex--
242254

@@ -245,7 +257,7 @@ func FromV1Image(v1Image v1.Image, config *Config) (*Image, error) {
245257
defer layerReader.Close()
246258

247259
tarReader := tar.NewReader(layerReader)
248-
layerSize, err := fillChainLayersWithFilesFromTar(outputImage, tarReader, layerDir, dirPath, chainLayersToFill)
260+
layerSize, err := fillChainLayersWithFilesFromTar(outputImage, tarReader, layerDir, chainLayersToFill)
249261
if err != nil {
250262
return fmt.Errorf("failed to fill chain layer with v1 layer tar: %w", err)
251263
}
@@ -255,7 +267,7 @@ func FromV1Image(v1Image v1.Image, config *Config) (*Image, error) {
255267
}()
256268

257269
if err != nil {
258-
return handleImageError(outputImage, err)
270+
return nil, handleImageError(outputImage, err)
259271
}
260272
}
261273

@@ -378,11 +390,11 @@ func addRootDirectoryToChainLayers(chainLayers []*chainLayer, extractDir string)
378390

379391
// handleImageError cleans up the image and returns the provided error. The image is cleaned up
380392
// regardless of the error, as the image is in an invalid state if an error is returned.
381-
func handleImageError(image *Image, err error) (*Image, error) {
393+
func handleImageError(image *Image, err error) error {
382394
if image != nil {
383395
_ = image.CleanUp()
384396
}
385-
return nil, err
397+
return err
386398
}
387399

388400
// validateHistory makes sure that the number of v1 layers matches the number of non-empty history
@@ -483,7 +495,7 @@ func initializeChainLayers(v1Layers []v1.Layer, history []v1.History, maxSymlink
483495
// fillChainLayersWithFilåesFromTar fills the chain layers with the files found in the tar. The
484496
// chainLayersToFill are the chain layers that will be filled with the files via the virtual
485497
// filesystem.
486-
func fillChainLayersWithFilesFromTar(img *Image, tarReader *tar.Reader, layerDir string, dirPath string, chainLayersToFill []*chainLayer) (int64, error) {
498+
func fillChainLayersWithFilesFromTar(img *Image, tarReader *tar.Reader, layerDir string, chainLayersToFill []*chainLayer) (int64, error) {
487499
if len(chainLayersToFill) == 0 {
488500
return 0, errors.New("no chain layers provided, this should not happen")
489501
}
@@ -551,10 +563,6 @@ func fillChainLayersWithFilesFromTar(img *Image, tarReader *tar.Reader, layerDir
551563
virtualPath = "/" + path.Join(dirname, basename)
552564
}
553565

554-
// realFilePath is where the file will be written to disk. filepath.Join will convert
555-
// any forward slashes to the appropriate OS specific path separator.
556-
realFilePath := filepath.Join(dirPath, filepath.FromSlash(cleanedFilePath))
557-
558566
// If the file already exists in the current chain layer, then skip it. This is done because
559567
// the tar file could be read multiple times to handle required symlinks.
560568
if currentChainLayer.fileNodeTree.Get(virtualPath) != nil {
@@ -564,9 +572,9 @@ func fillChainLayersWithFilesFromTar(img *Image, tarReader *tar.Reader, layerDir
564572
var newNode *fileNode
565573
switch header.Typeflag {
566574
case tar.TypeDir:
567-
newNode, err = img.handleDir(realFilePath, virtualPath, layerDir, header, isWhiteout)
575+
newNode, err = img.handleDir(virtualPath, layerDir, header, isWhiteout)
568576
case tar.TypeReg:
569-
newNode, err = img.handleFile(realFilePath, virtualPath, layerDir, tarReader, header, isWhiteout)
577+
newNode, err = img.handleFile(virtualPath, layerDir, tarReader, header, isWhiteout)
570578
case tar.TypeSymlink, tar.TypeLink:
571579
newNode, err = img.handleSymlink(virtualPath, layerDir, header, isWhiteout)
572580
default:
@@ -588,7 +596,7 @@ func fillChainLayersWithFilesFromTar(img *Image, tarReader *tar.Reader, layerDir
588596

589597
// If the virtual path has any directories and those directories have not been populated, then
590598
// populate them with file nodes.
591-
populateEmptyDirectoryNodes(virtualPath, layerDir, dirPath, chainLayersToFill)
599+
populateEmptyDirectoryNodes(virtualPath, layerDir, img.ExtractDir, chainLayersToFill)
592600

593601
// In each outer loop, a layer is added to each relevant output chainLayer slice. Because the
594602
// outer loop is looping backwards (latest layer first), we ignore any files that are already in
@@ -659,8 +667,9 @@ func (img *Image) handleSymlink(virtualPath, layerDir string, header *tar.Header
659667
}
660668

661669
// handleDir creates the directory specified by path, if it doesn't exist.
662-
func (img *Image) handleDir(realFilePath, virtualPath, layerDir string, header *tar.Header, isWhiteout bool) (*fileNode, error) {
663-
if _, err := os.Stat(realFilePath); err != nil {
670+
func (img *Image) handleDir(virtualPath, layerDir string, header *tar.Header, isWhiteout bool) (*fileNode, error) {
671+
realFilePath := filepath.Join(img.ExtractDir, layerDir, filepath.FromSlash(virtualPath))
672+
if _, err := img.root.Stat(filepath.Join(layerDir, filepath.FromSlash(virtualPath))); err != nil {
664673
if err := os.MkdirAll(realFilePath, dirPermission); err != nil {
665674
return nil, fmt.Errorf("failed to create directory with realFilePath %s: %w", realFilePath, err)
666675
}
@@ -681,15 +690,16 @@ func (img *Image) handleDir(realFilePath, virtualPath, layerDir string, header *
681690

682691
// handleFile creates the file specified by path, and then copies the contents of the tarReader into
683692
// the file.
684-
func (img *Image) handleFile(realFilePath, virtualPath, layerDir string, tarReader *tar.Reader, header *tar.Header, isWhiteout bool) (*fileNode, error) {
693+
func (img *Image) handleFile(virtualPath, layerDir string, tarReader *tar.Reader, header *tar.Header, isWhiteout bool) (*fileNode, error) {
694+
realFilePath := filepath.Join(img.ExtractDir, layerDir, filepath.FromSlash(virtualPath))
685695
parentDirectory := filepath.Dir(realFilePath)
686696
if err := os.MkdirAll(parentDirectory, dirPermission); err != nil {
687697
return nil, fmt.Errorf("failed to create parent directory %s: %w", parentDirectory, err)
688698
}
689-
// Write all files as read/writable by the current user, inaccessible by anyone else
690-
// Actual permission bits are stored in FileNode
691-
f, err := os.OpenFile(realFilePath, os.O_CREATE|os.O_RDWR, filePermission)
692699

700+
// Write all files as read/writable by the current user, inaccessible by anyone else. Actual
701+
// permission bits are stored in FileNode.
702+
f, err := img.root.OpenFile(filepath.Join(layerDir, filepath.FromSlash(virtualPath)), os.O_CREATE|os.O_RDWR, filePermission)
693703
if err != nil {
694704
return nil, err
695705
}

0 commit comments

Comments
 (0)