golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
123.92k stars 17.65k forks source link

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

Open rolandshoemaker opened 1 month ago

rolandshoemaker commented 1 month ago

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 commented 1 month ago

CC @golang/security as the OWNER.