Author: Manos Pitsidianakis [manos@pitsidianak.is]
Hash: d499c67eb42f49f129e25af9972c81b980a7f8af
Timestamp: Fri, 09 Jun 2023 13:33:49 +0000 (1 year ago)

+127 -12 +/-9 browse
web: make list description string safe for html if any owner is an admin
1diff --git a/core/src/models.rs b/core/src/models.rs
2index e4fe69a..f3c1e1d 100644
3--- a/core/src/models.rs
4+++ b/core/src/models.rs
5 @@ -58,6 +58,15 @@ impl<T> DbVal<T> {
6 }
7 }
8
9+ impl<T> std::borrow::Borrow<T> for DbVal<T>
10+ where
11+ T: Sized,
12+ {
13+ fn borrow(&self) -> &T {
14+ &self.0
15+ }
16+ }
17+
18 impl<T> std::ops::Deref for DbVal<T> {
19 type Target = T;
20 fn deref(&self) -> &T {
21 diff --git a/web/src/lists.rs b/web/src/lists.rs
22index c0197af..fa756db 100644
23--- a/web/src/lists.rs
24+++ b/web/src/lists.rs
25 @@ -118,6 +118,9 @@ pub async fn list(
26 url: ListPath(list.id.to_string().into()).to_crumb(),
27 },
28 ];
29+ let list_owners = db.list_owners(list.pk)?;
30+ let mut list_obj = MailingList::from(list.clone());
31+ list_obj.set_safety(list_owners.as_slice(), &state.conf.administrators);
32 let context = minijinja::context! {
33 canonical_url => ListPath::from(&list).to_crumb(),
34 page_title => &list.name,
35 @@ -128,7 +131,7 @@ pub async fn list(
36 months,
37 hists => &hist,
38 posts => posts_ctx,
39- list => Value::from_object(MailingList::from(list)),
40+ list => Value::from_object(list_obj),
41 current_user => auth.current_user,
42 user_context,
43 messages => session.drain_messages(),
44 @@ -196,11 +199,16 @@ pub async fn list_post(
45 url: ListPostPath(list.id.to_string().into(), msg_id.to_string()).to_crumb(),
46 },
47 ];
48+
49+ let list_owners = db.list_owners(list.pk)?;
50+ let mut list_obj = MailingList::from(list.clone());
51+ list_obj.set_safety(list_owners.as_slice(), &state.conf.administrators);
52+
53 let context = minijinja::context! {
54 canonical_url => ListPostPath(ListPathIdentifier::from(list.id.clone()), msg_id.to_string()).to_crumb(),
55 page_title => subject_ref,
56 description => &list.description,
57- list => Value::from_object(MailingList::from(list)),
58+ list => Value::from_object(list_obj),
59 pk => post.pk,
60 body => &body_text,
61 from => &envelope.field_from_to_string(),
62 @@ -300,6 +308,9 @@ pub async fn list_edit(
63 url: ListEditPath(ListPathIdentifier::from(list.id.clone())).to_crumb(),
64 },
65 ];
66+ let list_owners = db.list_owners(list.pk)?;
67+ let mut list_obj = MailingList::from(list.clone());
68+ list_obj.set_safety(list_owners.as_slice(), &state.conf.administrators);
69 let context = minijinja::context! {
70 canonical_url => ListEditPath(ListPathIdentifier::from(list.id.clone())).to_crumb(),
71 page_title => format!("Edit {} settings", list.name),
72 @@ -310,7 +321,7 @@ pub async fn list_edit(
73 post_count,
74 subs_count,
75 sub_requests_count,
76- list => Value::from_object(MailingList::from(list)),
77+ list => Value::from_object(list_obj),
78 current_user => auth.current_user,
79 messages => session.drain_messages(),
80 crumbs,
81 @@ -661,11 +672,14 @@ pub async fn list_subscribers(
82 url: ListEditSubscribersPath(list.id.to_string().into()).to_crumb(),
83 },
84 ];
85+ let list_owners = db.list_owners(list.pk)?;
86+ let mut list_obj = MailingList::from(list.clone());
87+ list_obj.set_safety(list_owners.as_slice(), &state.conf.administrators);
88 let context = minijinja::context! {
89 canonical_url => ListEditSubscribersPath(ListPathIdentifier::from(list.id.clone())).to_crumb(),
90 page_title => format!("Subscribers of {}", list.name),
91 subs,
92- list => Value::from_object(MailingList::from(list)),
93+ list => Value::from_object(list_obj),
94 current_user => auth.current_user,
95 messages => session.drain_messages(),
96 crumbs,
97 @@ -744,11 +758,13 @@ pub async fn list_candidates(
98 url: ListEditCandidatesPath(list.id.to_string().into()).to_crumb(),
99 },
100 ];
101+ let mut list_obj: MailingList = MailingList::from(list.clone());
102+ list_obj.set_safety(list_owners.as_slice(), &state.conf.administrators);
103 let context = minijinja::context! {
104 canonical_url => ListEditCandidatesPath(ListPathIdentifier::from(list.id.clone())).to_crumb(),
105 page_title => format!("Requests of {}", list.name),
106 subs,
107- list => Value::from_object(MailingList::from(list)),
108+ list => Value::from_object(list_obj),
109 current_user => auth.current_user,
110 messages => session.drain_messages(),
111 crumbs,
112 diff --git a/web/src/main.rs b/web/src/main.rs
113index df233ad..e80c06d 100644
114--- a/web/src/main.rs
115+++ b/web/src/main.rs
116 @@ -198,13 +198,14 @@ async fn root(
117 .earliest()
118 .map(|d| d.to_string())
119 });
120+ let list_owners = db.list_owners(list.pk)?;
121+ let mut list_obj = MailingList::from(list.clone());
122+ list_obj.set_safety(list_owners.as_slice(), &state.conf.administrators);
123 Ok(minijinja::context! {
124- name => &list.name,
125 newest,
126 posts => &posts,
127 months => &months,
128- description => &list.description.as_deref().unwrap_or_default(),
129- list => Value::from_object(MailingList::from(list.clone())),
130+ list => Value::from_object(list_obj),
131 })
132 })
133 .collect::<Result<Vec<_>, mailpot::Error>>()?;
134 diff --git a/web/src/minijinja_utils.rs b/web/src/minijinja_utils.rs
135index ce77da1..b7a3d65 100644
136--- a/web/src/minijinja_utils.rs
137+++ b/web/src/minijinja_utils.rs
138 @@ -21,6 +21,8 @@
139
140 use std::fmt::Write;
141
142+ use mailpot::models::ListOwner;
143+
144 use super::*;
145
146 mod compressed;
147 @@ -98,6 +100,29 @@ pub struct MailingList {
148 #[serde(serialize_with = "super::utils::to_safe_string_opt")]
149 pub archive_url: Option<String>,
150 pub inner: DbVal<mailpot::models::MailingList>,
151+ #[serde(default)]
152+ pub is_description_html_safe: bool,
153+ }
154+
155+ impl MailingList {
156+ /// Set whether it's safe to not escape the list's description field.
157+ ///
158+ /// If anyone can display arbitrary html in the server, that's bad.
159+ ///
160+ /// Note: uses `Borrow` so that it can use both `DbVal<ListOwner>` and
161+ /// `ListOwner` slices.
162+ pub fn set_safety<O: std::borrow::Borrow<ListOwner>>(
163+ &mut self,
164+ owners: &[O],
165+ administrators: &[String],
166+ ) {
167+ if owners.is_empty() || administrators.is_empty() {
168+ return;
169+ }
170+ self.is_description_html_safe = owners
171+ .iter()
172+ .any(|o| administrators.contains(&o.borrow().address));
173+ }
174 }
175
176 impl From<DbVal<mailpot::models::MailingList>> for MailingList {
177 @@ -124,6 +149,7 @@ impl From<DbVal<mailpot::models::MailingList>> for MailingList {
178 topics,
179 archive_url,
180 inner: val,
181+ is_description_html_safe: false,
182 }
183 }
184 }
185 @@ -181,9 +207,18 @@ impl minijinja::value::StructObject for MailingList {
186 "name" => Some(Value::from_serializable(&self.name)),
187 "id" => Some(Value::from_serializable(&self.id)),
188 "address" => Some(Value::from_serializable(&self.address)),
189+ "description" if self.is_description_html_safe => {
190+ self.description.as_ref().map_or_else(
191+ || Some(Value::from_serializable(&self.description)),
192+ |d| Some(Value::from_safe_string(d.clone())),
193+ )
194+ }
195 "description" => Some(Value::from_serializable(&self.description)),
196 "topics" => Some(Value::from_serializable(&self.topics)),
197 "archive_url" => Some(Value::from_serializable(&self.archive_url)),
198+ "is_description_html_safe" => {
199+ Some(Value::from_serializable(&self.is_description_html_safe))
200+ }
201 _ => None,
202 }
203 }
204 @@ -198,6 +233,7 @@ impl minijinja::value::StructObject for MailingList {
205 "description",
206 "topics",
207 "archive_url",
208+ "is_description_html_safe",
209 ][..],
210 )
211 }
212 @@ -782,4 +818,46 @@ mod tests {
213 0)(24, 0)(25, 0)(26, 0)(27, 0)(28, 0)(29, 0)(30, 0)"
214 );
215 }
216+
217+ #[test]
218+ fn test_list_html_safe() {
219+ let mut list = MailingList {
220+ pk: 0,
221+ name: String::new(),
222+ id: String::new(),
223+ address: String::new(),
224+ description: None,
225+ topics: vec![],
226+ archive_url: None,
227+ inner: DbVal(
228+ mailpot::models::MailingList {
229+ pk: 0,
230+ name: String::new(),
231+ id: String::new(),
232+ address: String::new(),
233+ description: None,
234+ topics: vec![],
235+ archive_url: None,
236+ },
237+ 0,
238+ ),
239+ is_description_html_safe: false,
240+ };
241+
242+ let mut list_owners = vec![ListOwner {
243+ pk: 0,
244+ list: 0,
245+ address: "admin@example.com".to_string(),
246+ name: None,
247+ }];
248+ let administrators = vec!["admin@example.com".to_string()];
249+ list.set_safety(&list_owners, &administrators);
250+ assert!(list.is_description_html_safe);
251+ list.set_safety::<ListOwner>(&[], &[]);
252+ assert!(list.is_description_html_safe);
253+ list.is_description_html_safe = false;
254+ list_owners[0].address = "user@example.com".to_string();
255+ list.set_safety(&list_owners, &administrators);
256+ assert!(!list.is_description_html_safe);
257+ }
258 }
259 diff --git a/web/src/settings.rs b/web/src/settings.rs
260index 033614b..20e38fb 100644
261--- a/web/src/settings.rs
262+++ b/web/src/settings.rs
263 @@ -302,6 +302,9 @@ pub async fn user_list_subscription(
264 },
265 ];
266
267+ let list_owners = db.list_owners(list.pk)?;
268+ let mut list = crate::minijinja_utils::MailingList::from(list);
269+ list.set_safety(list_owners.as_slice(), &state.conf.administrators);
270 let context = minijinja::context! {
271 page_title => "Subscription settings",
272 user => user,
273 diff --git a/web/src/templates/lists.html b/web/src/templates/lists.html
274index e1f8c45..a6d5698 100644
275--- a/web/src/templates/lists.html
276+++ b/web/src/templates/lists.html
277 @@ -5,7 +5,7 @@
278 <dl class="lists" aria-label="list of mailing lists">
279 {% for l in lists %}
280 <dt aria-label="mailing list name"><a href="{{ list_path(l.list.id) }}">{{ l.list.name }}</a></dt>
281- <dd><span aria-label="mailing list description"{% if not l.list.description %} class="no-description"{% endif %}>{{ l.list.description if l.list.description else "no description" }}</span> | {{ l.posts|length }} post{{ l.posts|length|pluralize("","s") }}{% if l.newest %} | <time datetime="{{ l.newest }}">{{ l.newest }}</time>{% endif %}{% if l.list.topics|length > 0 %}<br aria-hidden="true"><br aria-hidden="true"><span><em>Topics</em>:</span>&nbsp;{{ l.list.topics() }}{% endif %}</dd>
282+ <dd><span aria-label="mailing list description"{% if not l.list.description %} class="no-description"{% endif %}>{{ l.list.description if l.list.description else "<p>no description</p>"|safe }}</span><br />{{ l.posts|length }} post{{ l.posts|length|pluralize("","s") }}{% if l.newest %} | <time datetime="{{ l.newest }}">{{ l.newest }}</time>{% endif %}{% if l.list.topics|length > 0 %}<br aria-hidden="true"><br aria-hidden="true"><span><em>Topics</em>:</span>&nbsp;{{ l.list.topics() }}{% endif %}</dd>
283 {% endfor %}
284 </dl>
285 </div>
286 diff --git a/web/src/templates/lists/edit.html b/web/src/templates/lists/edit.html
287index 396bee5..02c3ef3 100644
288--- a/web/src/templates/lists/edit.html
289+++ b/web/src/templates/lists/edit.html
290 @@ -5,7 +5,11 @@
291 {{ list.name }} <a href="mailto:{{ list.address | safe }}"><code>{{ list.address }}</code></a>
292 </address>
293 {% if list.description %}
294- <p>{{ list.description }}</p>
295+ {% if list.is_description_html_safe %}
296+ {{ list.description|safe}}
297+ {% else %}
298+ <p>{{ list.description }}</p>
299+ {% endif %}
300 {% endif %}
301 {% if list.archive_url %}
302 <p><a href="{{ list.archive_url }}">{{ list.archive_url }}</a></p>
303 diff --git a/web/src/templates/lists/list.html b/web/src/templates/lists/list.html
304index 4844855..e6aef5c 100644
305--- a/web/src/templates/lists/list.html
306+++ b/web/src/templates/lists/list.html
307 @@ -5,7 +5,7 @@
308 <br aria-hidden="true">
309 {% endif %}
310 {% if list.description %}
311- <p title="mailing list description">List description: {{ list.description }}</p>
312+ <p title="mailing list description">{{ list.description }}</p>
313 {% else %}
314 <p title="mailing list description">No list description.</p>
315 {% endif %}
316 @@ -17,12 +17,14 @@
317 <input type="hidden" name="list_pk", value="{{ list.pk }}">
318 <input type="submit" name="unsubscribe" value="Unsubscribe as {{ current_user.address }}">
319 </form>
320+ <br />
321 {% else %}
322 <form method="post" action="{{ settings_path() }}" class="settings-form">
323 <input type="hidden" name="type", value="subscribe">
324 <input type="hidden" name="list_pk", value="{{ list.pk }}">
325 <input type="submit" name="subscribe" value="Subscribe as {{ current_user.address }}">
326 </form>
327+ <br />
328 {% endif %}
329 {% endif %}
330 {% if preamble %}
331 diff --git a/web/src/templates/settings_subscription.html b/web/src/templates/settings_subscription.html
332index cd2d708..abe7289 100644
333--- a/web/src/templates/settings_subscription.html
334+++ b/web/src/templates/settings_subscription.html
335 @@ -4,7 +4,9 @@
336 <address>
337 {{ list.name }} <a href="mailto:{{ list.address | safe }}"><code>{{ list.address }}</code></a>
338 </address>
339- {% if list.description %}
340+ {% if list.is_description_html_safe %}
341+ {{ list.description|safe}}
342+ {% else %}
343 <p>{{ list.description }}</p>
344 {% endif %}
345 {% if list.archive_url %}