gostatic-analysisgolint

G110: Potential DoS vulnerability via decompression bomb (gosec)


I'm getting the following golintci message:

testdrive/utils.go:92:16: G110: Potential DoS vulnerability via decompression bomb (gosec)
    if _, err := io.Copy(targetFile, fileReader); err != nil {
                 ^

Read the corresponding CWE and I'm not clear on how this is expected to be corrected.

Please offer pointers.

func unzip(archive, target string) error {
    reader, err := zip.OpenReader(archive)
    if err != nil {
        return err
    }

    for _, file := range reader.File {
        path := filepath.Join(target, file.Name) // nolint: gosec
        if file.FileInfo().IsDir() {
            if err := os.MkdirAll(path, file.Mode()); err != nil {
                return err
            }
            continue
        }

        fileReader, err := file.Open()
        if err != nil {
            return err
        }
        defer fileReader.Close() // nolint: errcheck

        targetFile, err := os.OpenFile(path, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, file.Mode())
        if err != nil {
            return err
        }
        defer targetFile.Close() // nolint: errcheck

        if _, err := io.Copy(targetFile, fileReader); err != nil {
            return err
        }
    }

    return nil
}

Solution

  • Based on various pointers provided, replaced

    if _, err := io.Copy(targetFile, fileReader); err != nil {
      return err
    }
    

    with

    for {
      _, err := io.CopyN(targetFile, fileReader, 1024)
      if err != nil {
        if err == io.EOF {
          break
        }
        return err
      }
    }
    

    PS while this helps memory footprint, this wouldn't help a DDOS attack copying very long and/or infinite stream ...