Copy Method update to avoid DoS via decompression bomb

[mohanmanikanta2299]

Updated the code by using io.CopyN method instead of io.Copy to mitigate the vulnerability.

This change will help to copy the contents of the file in reasonably sized chunks by using the io.CopyN method in a loop. Added logic to limit the number of chunks copied in this loop in order to prevent an excessive number of iterations that could lead to denial of service.

[dduzgun-security]

Seems like the counter is incremented only after a full chunk is copied which could allow going over the max of 400MB by 4MB. Also, the range for the sizing seems a bit excessive for the files go-slug deals with, we could try with 50MB as a max.

[dduzgun-security]

Also, was func CopyN(dst Writer, src Reader, n int64) (written int64, err error) not working?

[eastebry]

Hey folks! Just a heads-up, I think we need to be pretty careful with this change. This code path is used for extracting slugs within Terraform, and it seems plausible that these will exceed 10MB.

Generally speaking, I'm guessing the risk here is fairly low because (as far as I know, I could be wrong), slugs are mostly unpacked in single-tenant environments (either in Terraform core, or within the sandbox environment of Terraform cloud). Given this, I think we need to carefully consider if this security improvement is worth the potential user impact/risk.

Before merging this, can we please do the following:

• Talk with all consumers of go-slug (Terraform code, registry, cloud, and potentially more) to discuss what would be a reasonable limit, or if any limit we would impose could pose problems .
• If we enforce a specific limit, ensure that it is explained in our product documentation, and called out in any release notes.

[dduzgun-security]

Considering all the implications here and the customers use cases which could go from unpacking files of MB to GB in size based out of feedbacks from the TF team, I think we should close/dismiss this issue to not cause any incident. Setting a hard limit would cause potential problems in the future for specific customer use case. I'll go ahead an close this alert.