github-vet / github-vet/rangeloop-pointer-findings
containers/buildah: add.go; 120 LoC
- Dominant language
- No language data
- Stars
- 0
- Forks
- 0
- PR merge metrics
- PR metrics pending
Description
Found a possible issue in [containers/buildah](https://www.github.com/containers/buildah) at [add.go](https://github.com/containers/buildah/blob/02b914b1c424c151d0a32192876a37daf5927981/add.go#L396-L515)
Below is the message reported by the analyzer for this snippet of code. Beware that the analyzer only reports the first issue it finds, so please do not limit your consideration to the contents of the below message.
> range-loop variable glob used in defer or goroutine at line 456
[Click here to see the code in its original context.](https://github.com/containers/buildah/blob/02b914b1c424c151d0a32192876a37daf5927981/add.go#L396-L515)
Click here to show the 120 line(s) of Go which triggered the analyzer.
```go
for _, glob := range localSourceStat.Globbed {
rel, err := filepath.Rel(contextDir, glob)
if err != nil {
return errors.Wrapf(err, "error computing path of %q relative to %q", glob, contextDir)
}
if strings.HasPrefix(rel, ".."+string(os.PathSeparator)) {
return errors.Errorf("possible escaping context directory error: %q is outside of %q", glob, contextDir)
}
// Check for dockerignore-style exclusion of this item.
if rel != "." {
excluded, err := pm.Matches(filepath.ToSlash(rel)) // nolint:staticcheck
if err != nil {
return errors.Wrapf(err, "error checking if %q(%q) is excluded", glob, rel)
}
if excluded {
// non-directories that are excluded are excluded, no question, but
// directories can only be skipped if we don't have to allow for the
// possibility of finding things to include under them
globInfo := localSourceStat.Results[glob]
if !globInfo.IsDir || !includeDirectoryAnyway(rel, pm) {
continue
}
}
} else {
// Make sure we don't trigger a "copied nothing" error for an empty context
// directory if we were told to copy the context directory itself. We won't
// actually copy it, but we need to make sure that we don't produce an error
// due to potentially not having anything in the tarstream that we passed.
itemsCopied++
}
st := localSourceStat.Results[glob]
pipeReader, pipeWriter := io.Pipe()
wg.Add(1)
go func() {
renamedItems := 0
writer := io.WriteCloser(pipeWriter)
if renameTarget != "" {
writer = newTarFilterer(writer, func(hdr *tar.Header) (bool, bool, io.Reader) {
hdr.Name = renameTarget
renamedItems++
return false, false, nil
})
}
writer = newTarFilterer(writer, func(hdr *tar.Header) (bool, bool, io.Reader) {
itemsCopied++
return false, false, nil
})
getOptions := copier.GetOptions{
UIDMap: srcUIDMap,
GIDMap: srcGIDMap,
Excludes: options.Excludes,
ExpandArchives: extract,
ChownDirs: chownDirs,
ChmodDirs: nil,
ChownFiles: chownFiles,
ChmodFiles: nil,
StripSetuidBit: options.StripSetuidBit,
StripSetgidBit: options.StripSetgidBit,
StripStickyBit: options.StripStickyBit,
}
getErr = copier.Get(contextDir, contextDir, getOptions, []string{glob}, writer)
closeErr = writer.Close()
if renameTarget != "" && renamedItems > 1 {
renameErr = errors.Errorf("internal error: renamed %d items when we expected to only rename 1", renamedItems)
}
wg.Done()
}()
wg.Add(1)
go func() {
if st.IsDir {
b.ContentDigester.Start("dir")
} else {
b.ContentDigester.Start("file")
}
hashCloser := b.ContentDigester.Hash()
hasher := io.Writer(hashCloser)
if options.Hasher != nil {
hasher = io.MultiWriter(hasher, options.Hasher)
}
if options.DryRun {
_, putErr = io.Copy(hasher, pipeReader)
} else {
putOptions := copier.PutOptions{
UIDMap: destUIDMap,
GIDMap: destGIDMap,
DefaultDirOwner: chownDirs,
DefaultDirMode: nil,
ChownDirs: nil,
ChmodDirs: nil,
ChownFiles: nil,
ChmodFiles: nil,
IgnoreDevices: rsystem.RunningInUserNS(),
}
putErr = copier.Put(mountPoint, extractDirectory, putOptions, io.TeeReader(pipeReader, hasher))
}
hashCloser.Close()
pipeReader.Close()
wg.Done()
}()
wg.Wait()
if getErr != nil {
getErr = errors.Wrapf(getErr, "error reading %q", src)
}
if closeErr != nil {
closeErr = errors.Wrapf(closeErr, "error closing %q", src)
}
if renameErr != nil {
renameErr = errors.Wrapf(renameErr, "error renaming %q", src)
}
if putErr != nil {
putErr = errors.Wrapf(putErr, "error storing %q", src)
}
multiErr = multierror.Append(getErr, closeErr, renameErr, putErr)
if multiErr != nil && multiErr.ErrorOrNil() != nil {
if len(multiErr.Errors) > 1 {
return multiErr.ErrorOrNil()
}
return multiErr.Errors[0]
}
}
```
Leave a reaction on this issue to contribute to the project by classifying this instance as a **Bug** :-1:, **Mitigated** :+1:, or **Desirable Behavior** :rocket:
See the descriptions of the classifications [here](https://github.com/github-vet/rangeclosure-findings#how-can-i-help) for more information.
commit ID: 02b914b1c424c151d0a32192876a37daf5927981
Contributor guide
No contributing guide indexed for this repository
Research direction
Inspect containers/buildah/add.go at lines 396-515, especially the goroutines inside the loop over localSourceStat.Globbed. Determine whether the reported range-loop capture affects behavior in this context, then classify the finding as Bug, Mitigated, or Desirable Behavior using the issue's reaction options.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- devops
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Stale
- Clarity
- Needs clarification
- Newbie friendliness
- 30/100