a73x

e242aac3

Fix stored XSS in the web UI branch selectors

a73x   2026-08-09 14:08

Commit message
Fix stored XSS in the web UI branch selectors
src/server/http/templates/blob.html
Old New
@@ -5,9 +5,9 @@
5 {% block content %} 5 {% block content %}
6 <h2>{{ file_path }}</h2> 6 <h2>{{ file_path }}</h2>
7 <p style="color: #666;">Ref: 7 <p style="color: #666;">Ref:
8 <select class="mono" onchange="location.href='/{{ repo_name }}/blob/' + this.value + '/{{ file_path }}'"> 8 <select class="mono" onchange="location.href=this.value">
9 {% for b in branches %} 9 {% for b in branches %}
10 <option value="{{ b }}"{% if *b == ref_name %} selected{% endif %}>{{ b }}</option> 10 <option value="/{{ repo_name }}/blob/{{ b }}/{{ file_path }}"{% if *b == ref_name %} selected{% endif %}>{{ b }}</option>
11 {% endfor %} 11 {% endfor %}
12 </select> 12 </select>
13 &nbsp; Size: {{ size_display }}</p> 13 &nbsp; Size: {{ size_display }}</p>
src/server/http/templates/commits.html
Old New
@@ -4,9 +4,9 @@
4 4
5 {% block content %} 5 {% block content %}
6 <h2>Commits: 6 <h2>Commits:
7 <select onchange="location.href='/{{ repo_name }}/commits/' + this.value"> 7 <select onchange="location.href=this.value">
8 {% for b in branches %} 8 {% for b in branches %}
9 <option value="{{ b }}"{% if *b == ref_name %} selected{% endif %}>{{ b }}</option> 9 <option value="/{{ repo_name }}/commits/{{ b }}"{% if *b == ref_name %} selected{% endif %}>{{ b }}</option>
10 {% endfor %} 10 {% endfor %}
11 </select> 11 </select>
12 </h2> 12 </h2>
src/server/http/templates/tree.html
Old New
@@ -4,9 +4,9 @@
4 4
5 {% block content %} 5 {% block content %}
6 <h2> 6 <h2>
7 <select onchange="location.href='/{{ repo_name }}/tree/' + this.value + '{% if !path_display.is_empty() %}/{{ path_display }}{% endif %}'"> 7 <select onchange="location.href=this.value">
8 {% for b in branches %} 8 {% for b in branches %}
9 <option value="{{ b }}"{% if *b == ref_name %} selected{% endif %}>{{ b }}</option> 9 <option value="/{{ repo_name }}/tree/{{ b }}{% if !path_display.is_empty() %}/{{ path_display }}{% endif %}"{% if *b == ref_name %} selected{% endif %}>{{ b }}</option>
10 {% endfor %} 10 {% endfor %}
11 </select> 11 </select>
12 {% if !path_display.is_empty() %} / {{ path_display }}{% endif %} 12 {% if !path_display.is_empty() %} / {{ path_display }}{% endif %}
tests/server_behavior_test.rs
Old New
@@ -188,3 +188,77 @@ fn missing_readme_does_not_break_overview_page() {
188 // The rest of the page still renders. 188 // The rest of the page still renders.
189 assert!(overview.body.contains("Open Patches") || overview.body.contains("Recent Commits")); 189 assert!(overview.body.contains("Open Patches") || overview.body.contains("Recent Commits"));
190 } 190 }
191
192 /// Every `onchange="..."` attribute value in `body`.
193 ///
194 /// An inline event-handler attribute is HTML-decoded *before* its contents are
195 /// compiled as JavaScript, so askama's `'` -> `&#39;` escaping does not survive
196 /// into the JS parse. Any server-controlled data interpolated into one of these
197 /// is a script-injection sink, which is why the assertions below demand the
198 /// handlers carry no data at all rather than merely checking for a payload.
199 fn onchange_handlers(body: &str) -> Vec<String> {
200 let mut handlers = Vec::new();
201 let mut rest = body;
202 while let Some(start) = rest.find("onchange=\"") {
203 rest = &rest[start + "onchange=\"".len()..];
204 let end = rest.find('"').expect("unterminated onchange attribute");
205 handlers.push(rest[..end].to_string());
206 rest = &rest[end..];
207 }
208 handlers
209 }
210
211 /// The branch-selector handlers must be constant: the destination belongs on
212 /// each `<option value>`, an ordinary attribute where HTML escaping is
213 /// sufficient, never spliced into the JS source itself.
214 #[test]
215 fn branch_selectors_never_interpolate_data_into_inline_javascript() {
216 let harness = ServerHarness::new("behavior-selector-xss");
217
218 // A single quote is legal in a git path. Interpolated into a JS string
219 // literal it closes the string and the rest of the name becomes code.
220 harness.work_repo().commit_file(
221 "d'-alert(1)-'/q'-alert(2)-'.txt",
222 "payload\n",
223 "add file with a quote in its name",
224 );
225 harness.push_head();
226
227 let repo = harness.repo_name();
228 let pages = [
229 format!("/{}/commits", repo),
230 format!("/{}/tree", repo),
231 format!("/{}/tree/main/d%27-alert(1)-%27", repo),
232 format!(
233 "/{}/blob/main/d%27-alert(1)-%27/q%27-alert(2)-%27.txt",
234 repo
235 ),
236 ];
237
238 for page in &pages {
239 let response = harness.get_ok(page);
240 let handlers = onchange_handlers(&response.body);
241 assert!(
242 !handlers.is_empty(),
243 "expected a branch selector on {page}, got none:\n{}",
244 response.body
245 );
246 for handler in &handlers {
247 assert_eq!(
248 handler, "location.href=this.value",
249 "inline handler on {page} carries interpolated data: {handler}"
250 );
251 }
252 }
253
254 // The selector must still work: the option carries the real destination.
255 let tree = harness.get_ok(&format!("/{}/tree/main/d%27-alert(1)-%27", repo));
256 assert!(
257 tree.body.contains(&format!(
258 "value=\"/{}/tree/main/d&#39;-alert(1)-&#39;\"",
259 repo
260 )),
261 "option value missing or wrong:\n{}",
262 tree.body
263 );
264 }