G110: потенциальная уязвимость DoS из-за декомпрессионной бомбы (gosec)

Я получаю следующее golintci сообщение:

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

Прочитайте соответствующий CWE, и я не понимаю, как это должно исправить.

Пожалуйста, предложите указатели.

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
}

person gliptak    schedule 30.04.2021    source источник


Ответы (3)


Предупреждение, которое вы получаете, исходит из правила, представленного в gosec.

Правило специально определяет использование io.Copy при распаковке файлов.

Это потенциальная проблема, поскольку io.Copy:

копирует из src в dst до тех пор, пока не будет достигнут EOF на src или не произойдет ошибка.

Таким образом, вредоносная полезная нагрузка может привести к тому, что ваша программа распаковает неожиданно большой объем данных и выйдет из памяти, что приведет к отказу в обслуживании, как указано в предупреждающем сообщении.

В частности, gosec проверит (источник ) AST вашей программы и предупредит вас об использовании io.Copy или io.CopyBuffer вместе с любым из следующего:

  • "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

Использование io.CopyN удаляет предупреждение, поскольку (цитата) оно копирует n байтов (или до тех пор, пока error) из src в dst, что дает вам (разработчику программы) возможность контролировать, сколько байт копировать. Таким образом, вы можете передать произвольно большое значение n, которое вы устанавливаете на основе доступных ресурсов вашего приложения, или копировать по частям.

person blackgreen    schedule 30.04.2021

На основе различных указателей, замененных

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

с

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

PS, хотя это помогает уменьшить объем памяти, это не поможет при DDOS-атаке, копирующей очень длинный и/или бесконечный поток...

person gliptak    schedule 04.05.2021
comment
Я думаю, что преимущество фрагментации заключается в том, что вы можете иметь дополнительную логику в цикле для смягчения последствий атаки декомпрессии, но это, конечно, зависит от специфики вашего приложения. - person blackgreen; 05.05.2021

Предполагая, что вы работаете со сжатыми данными, вам нужно использовать io.CopyN.
Вы можете попробовать обходной путь с флагом --nocompress. Но это приведет к тому, что данные будут включены без сжатия.

См. следующий PR и связанную с ним проблему: https://github.com/go-bindata/go-bindata/pull/50

person advay rajhansa    schedule 30.04.2021