Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Backport of Build with Go 1.19.3 into v1.3 #32145

Merged

Conversation

teamterraform
Copy link
Contributor

Backport

This PR is auto-generated from #32135 to be assessed for backporting due to the inclusion of the label 1.3-backport.

The below text is copied from the body of the original PR.


This includes a small selection of security-related fixes which do not urgently impact Terraform's behavior but do close some potential avenues for unbounded resource usage or misbehavior with malicious input.

I've included some details below on each of the security-related fixes included in these updates (both Go 1.19.2 and Go 1.19.3) along with justifications for why they are not urgent issues in the context of Terraform, and so can therefore be released naturally in the next patch release rather than in an out-of-order security release.

Unbounded memory usage for crafted tar archive headers

golang/go#54853

Terraform accepts tar archives over HTTP as one of the many possible module source types. In principle an attacker could publish a module package in a tar archive with a crafted header and consume significant memory on the system where Terraform is running.

The impact of this is limited because it would require a Terraform configuration to already be depending on an untrustworthy module source referring to a tar archive. Most consumption of third-party modules is either indirect via the public Terraform Registry (which wraps GitHub) or directly from Git repositories, neither of which cause Terraform to retrieve and extract arbitrary tar archives that can be controlled by an attacker.

Unbounded memory usage for crafted patterns to Regular Expression functions

golang/go#55949

Terraform's built-in functions regex, regexall, and replace all rely on the Go standard library regular expression compiler for their work. In principle an attacker could publish a module which includes a crafted regular expression pattern which consumes 40,000 times more memory than the source input length, thereby consuming significant memory on the system where Terraform is running.

Terraform only evaluates functions at points where various other kinds of system access are already possible, and so this is a new example of an existing possible attack vector: Terraform modules under evaluation are effectively arbitrary code, and there are other existing ways for an untrusted module to take undesirable actions and so users should already be taking care to review any third-party modules they depend on.

However, it is true that those reviewing modules are unlikely to consider a regular expression pattern as a possible attack vector and so patching this will avoid that surprising possibility in future versions of Terraform.

Smuggling of Environment Variables on Windows systems

golang/go#56284

This bug in the Go standard library in principle allows making a child process of the Terraform process interpret its environment variable table differently than the Terraform process would have interpreted it. Terraform launches child processes for provider plugins, version control clients like git, and for any configured credentials helper.

There are no known situations where it would be a productive attack vector to "smuggle" an environment variable to a child process in this way, because Terraform CLI does not perform any screening of the environment passed to child processes and so it would be equivalent to just set the same environment variable directly, without exploiting this bug.


Because none of these problems seem to be significant issues for Terraform usage in particular, I propose that we record them in the changelog only as a series of normal bug fix entries, and not call them out under an explicit security header as we might do for a more significant advisory:

  • When installing remote module packages delivered in tar format, Terraform now limits the tar header block size to 1MiB to avoid unbounded memory usage for maliciously-crafted module packages. [Build with Go 1.19.3 #32135]
  • Terraform will now reject excessively-complex regular expression patterns passed to the regex, regexall, and replace functions, to avoid unbounded memory usage for maliciously-crafted patterns. This change should not affect any reasonable patterns intended for practical use. [Build with Go 1.19.3 #32135]
  • Terraform on Windows now rejects invalid environment variables whose values contain the NUL character when propagating environment variables to a child process such as a provider plugin. Previously Terraform would incorrectly treat that character as a separator between two separate environment variables. [Build with Go 1.19.3 #32135]

@teamterraform teamterraform force-pushed the backport/f-build-go1.19.3/largely-peaceful-grouper branch from 648a18f to 595bff1 Compare November 2, 2022 15:59
@apparentlymart apparentlymart merged commit 6827783 into v1.3 Nov 2, 2022
@github-actions
Copy link

github-actions bot commented Nov 2, 2022

Reminder for the merging maintainer: if this is a user-visible change, please update the changelog on the appropriate release branch.

@github-actions
Copy link

github-actions bot commented Dec 3, 2022

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 3, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants