G110: Potential DoS vulnerability via decompression bomb (gosec)

Question

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
}

Answer 1

The warning you get comes from a rule provided in gosec.

The rule specifically detects usage of io.Copy on file decompression.

This is a potential issue because io.Copy:

copies from src to dst until either EOF is reached on src or an error occurs.

So, a malicious payload might cause your program to decompress an unexpectedly big amount of data and go out of memory, causing denial of service as mentioned in the warning message.

In particular, gosec will check (source) the AST of your program and warn you about usage of io.Copy or io.CopyBuffer together with any one of the following:

• "compress/gzip".NewReader
• "compress/zlib".NewReader or NewReaderDict
• "compress/bzip2".NewReader
• "compress/flate".NewReader or NewReaderDict
• "compress/lzw".NewReader
• "archive/tar".NewReader
• "archive/zip".NewReader
• "*archive/zip".File.Open

Using io.CopyN removes the warning because (quote) it "copies n bytes (or until an error) from src to dst", thus giving you (the program writer) control of how many bytes to copy. So you could pass an arbitrarily large n that you set based on the available resources of your application, or copy in chunks.

Answer 2

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 ...