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

html/template: template Parse/Execute escaping race #69404

Open
rolandshoemaker opened this issue Sep 11, 2024 · 1 comment
Open

html/template: template Parse/Execute escaping race #69404

rolandshoemaker opened this issue Sep 11, 2024 · 1 comment
Labels
NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Milestone

Comments

@rolandshoemaker
Copy link
Member

Template.Execute and Template.Parse can race, causing template escaping state to become out-of-sync.

Template.Parse allows re-parsing a template, but is documented as not being callable after the first call to Template.Execute. It checks this by seeing if the template has been escaped (by inspecting Template.namespace.escaped). When Template.Execute is first called, it will check if the template is escaped, and escapes it if it was not already. It then sets Template.namespace.escaped to indicate the template is escaped.

Template.namespace.escaped is protected with a lock, but Template.Parse doesn't hold this lock for the duration of its execution, taking the lock to initially check Template.namespace.escaped and then taking it again later to populate the re-parsed templates. If Template.Execute is called between these two steps, it takes the lock, escapes the template and sets Template.namespace.escaped. Template.Parse will then re-take the lock, overwrite the escaped template, but not unset Template.namespace.escaped, causing subsequent calls to Template.Execute to execute an unescaped template.

Since this requires concurrent calls to Parse and Execute, and can only be triggered on the initial call to Execute, this is extremely hard to exploit, but it is cleanly incorrect behavior.

This issue was initially reported to us by Jakob Ackermann.

@timothy-king timothy-king added this to the Backlog milestone Sep 12, 2024
@timothy-king timothy-king added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Sep 12, 2024
@timothy-king
Copy link
Contributor

CC @golang/security as the OWNER.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Projects
None yet
Development

No branches or pull requests

2 participants