operator/pkg/util: Curious about Adding a Data Limit Check in ioCopyN?

[mohamedawnallah]

Description

Currently, the ioCopyN function does not enforce a limit on the amount of data it processes from the tar.Reader. This can lead to potential Denial of Service (DoS) vulnerabilities through decompression bombs, where maliciously crafted tar files could cause excessive resource consumption.

karmada/operator/pkg/util/util.go

Lines 155 to 166 in 6e41d9b

	// ioCopyN fix Potential DoS vulnerability via decompression bomb.
	func ioCopyN(outFile *os.File, tr *tar.Reader) error {
	for {
	if _, err := io.CopyN(outFile, tr, 1024); err != nil {
	if errors.Is(err, io.EOF) {
	break
	}
	return err
	}
	}
	return nil
	}

Proposed Changes

Implement a data limit check in the ioCopyN function to prevent excessive data processing. This can be achieved by adding the following code to track the total number of bytes written and enforce a maximum size limit:

totalWritten += n
if totalWritten > maxSize {
    return fmt.Errorf("data limit exceeded: total written %d bytes, maximum allowed %d bytes", totalWritten, maxSize)
}

What do you think ? 🤔

[prakrit55]

hey @mohamedawnallah, if its open I would like to fix it

[mohamedawnallah]

Hey @prakrit55, I've submitted an improvement suggestion that hasn't been confirmed yet. Could @XiShanYongYe-Chang and @zhzhuang-zju also review it?

[zhzhuang-zju]

@mohamedawnallah thanks for raising this issue. I have two questions regarding this:

• Is this a known issue? Is it a widely accepted practice to add a data limit check?
• What should the maxSize be set to?