Skip to content

Conversation

austinderek
Copy link

Introduce a hard limit on the number of commands in a single
pipeline. Return a parse error when exceeded. This bounds
worst-case time and memory amplification for pathologically
long pipelines, mitigating issues when when templates are not
fully trusted.

Fixes golang#75231


🔄 This is a mirror of upstream PR golang#75233

nicholashusin and others added 5 commits August 31, 2025 05:18
…mpty

In 1.21 ServeMux, we had a special-case to skip redirection when a given
path is empty for CONNECT requests:
https://go.googlesource.com/go/+/refs/tags/go1.24.4/src/net/http/servemux121.go#205.

This special case seems to not have been carried over to 1.22 ServeMux.
This causes needless redirection, which this CL fixes.

Fixes golang#74422

Change-Id: I3cc5b4d195ab0591a9139225b632cbe17f4290db
Reviewed-on: https://go-review.googlesource.com/c/go/+/699915
Reviewed-by: Sean Liao <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>
Auto-Submit: Sean Liao <[email protected]>
Reviewed-by: Damien Neil <[email protected]>
The implementation here needs to be consistent with ssa.OpLOONG64LoweredAtomicCas{32,64},
which was ignored in CL 613396.

Change-Id: I72e8d2318e0c1935cc3a35ab5098f8a84e48bcd5
Reviewed-on: https://go-review.googlesource.com/c/go/+/699395
Reviewed-by: Keith Randall <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>
Reviewed-by: Cherry Mui <[email protected]>
Reviewed-by: sophie zhao <[email protected]>
Reviewed-by: Meidan Li <[email protected]>
…s support

Go asm syntax:
	MOVWP		4(R4), R5
	MOVVP		8(R4), R5
	MOVWP		R4, 12(R5)
	MOVVP		R4, 16(R5)

Equivalent platform assembler syntax:
	ldptr.w		r5, r4, $1
	ldptr.d		r5, r4, $2
	stptr.w		r4, r5, $3
	stptr.d		r4, r5, $4

Change-Id: I50a341cee2d875cb7c5da9db08b23799c9dc6c64
Reviewed-on: https://go-review.googlesource.com/c/go/+/699055
Reviewed-by: abner chenc <[email protected]>
Reviewed-by: Meidan Li <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>
Reviewed-by: Carlos Amedee <[email protected]>
Reviewed-by: Cherry Mui <[email protected]>
Change-Id: Id43ee4353d4bac96627f8b0f54545cdd3d2a1d1b
Reviewed-on: https://go-review.googlesource.com/c/go/+/699695
Reviewed-by: Cherry Mui <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>
Reviewed-by: Carlos Amedee <[email protected]>
Reviewed-by: abner chenc <[email protected]>
Introduce a hard limit on the number of commands in a single
pipeline. Return a parse error when exceeded. This bounds
worst-case time and memory amplification for pathologically
long pipelines, mitigating issues when when templates are not
fully trusted.

Fixes golang#75231
@austinderek austinderek force-pushed the master branch 25 times, most recently from 2a7f1d4 to 37c78b5 Compare September 3, 2025 07:02
@austinderek austinderek force-pushed the master branch 27 times, most recently from ca0e035 to 37c78b5 Compare September 16, 2025 10:02
Copy link

staging bot commented Sep 16, 2025

HackerOne Code Security Review

🟢 Scan Complete: 2 Issue(s)
🟠 Validation Complete: One or more Issues looked potentially actionable, so this was escalated to our network of engineers for manual review. Once this is complete you'll see an update posted.

Here's how the code changes were interpreted and info about the tools used for scanning.

📖 Summary of Changes

The changes in the parse.go file introduce a new variable maxPipelineCmds to control the maximum number of commands allowed in a pipeline. A validation check has been added to the pipeline method to enforce this limit, potentially preventing the creation of overly complex or resource-intensive pipeline structures.

File Summary
src/text/template/parse/parse.go Added a new variable maxPipelineCmds to limit the number of commands in a pipeline, with a check added in the pipeline method to prevent excessively long pipelines.
ℹ️ Issues Detected

NOTE: These may not require action!

Below are unvalidated results from the Analysis Tools that ran during the latest scan for transparency. We investigate each of these for accuracy and relevance before surfacing them as a potential problem.

How will I know if something is a problem?
When validation completes, any concerns that warrant attention prior to merge will be posted as inline comments. These will show up in 2 ways:

  • Expert review (most cases): Issues will be posted by experts who manually reviewed and validated them. These are real HackerOne engineers (not bots) reviewing through an integrated IDE-like tool. You can communicate with them like any other reviewer. They'll stay assigned and get notified with commit & comment updates.
  • Automatically: In cases where our validation checks have highest confidence the problem is legitimate and urgent. These will include a description of contextual reasoning why & actionable next steps.
File & Line Issue
src/text/template/parse/parse.go Line 52 The maxPipelineCmds limit of 100,000 commands per pipeline may be insufficient to prevent resource exhaustion attacks. An attacker could craft templates with pipelines approaching this limit to consume excessive CPU and memory during parsing and execution. Consider reducing this limit to a more conservative value (e.g., 1,000-10,000) based on legitimate use cases, or implement additional resource controls such as parsing timeouts.
src/text/template/parse/parse.go Line 520 The pipeline length check occurs after each command is added to the pipeline, allowing an attacker to create exactly maxPipelineCmds commands before the limit is enforced. This could still result in significant resource consumption. The check should be performed before adding the command to prevent reaching the exact limit: 'if len(pipe.Cmds) >= maxPipelineCmds { t.errorf("pipeline too long") }'.
🧰 Analysis tools
⏱️ Latest scan covered changes up to commit 97e6001 (latest)

@austinderek austinderek deleted the fix/text-template-max-pipeline-commands branch September 16, 2025 10:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants