web: make list description string safe for html if any owner is an admin

core/src/models.rs

@@ -58,6 +58,15 @@ impl<T> DbVal<T> {
     }
 }
 
+impl<T> std::borrow::Borrow<T> for DbVal<T>
+where
+    T: Sized,
+{
+    fn borrow(&self) -> &T {
+        &self.0
+    }
+}
+
 impl<T> std::ops::Deref for DbVal<T> {
     type Target = T;
     fn deref(&self) -> &T {

web/src/lists.rs

@@ -118,6 +118,9 @@ pub async fn list(
             url: ListPath(list.id.to_string().into()).to_crumb(),
         },
     ];
+    let list_owners = db.list_owners(list.pk)?;
+    let mut list_obj = MailingList::from(list.clone());
+    list_obj.set_safety(list_owners.as_slice(), &state.conf.administrators);
     let context = minijinja::context! {
         canonical_url => ListPath::from(&list).to_crumb(),
         page_title => &list.name,
@@ -128,7 +131,7 @@ pub async fn list(
         months,
         hists => &hist,
         posts => posts_ctx,
-        list => Value::from_object(MailingList::from(list)),
+        list => Value::from_object(list_obj),
         current_user => auth.current_user,
         user_context,
         messages => session.drain_messages(),
@@ -196,11 +199,16 @@ pub async fn list_post(
             url: ListPostPath(list.id.to_string().into(), msg_id.to_string()).to_crumb(),
         },
     ];
+
+    let list_owners = db.list_owners(list.pk)?;
+    let mut list_obj = MailingList::from(list.clone());
+    list_obj.set_safety(list_owners.as_slice(), &state.conf.administrators);
+
     let context = minijinja::context! {
         canonical_url => ListPostPath(ListPathIdentifier::from(list.id.clone()), msg_id.to_string()).to_crumb(),
         page_title => subject_ref,
         description => &list.description,
-        list => Value::from_object(MailingList::from(list)),
+        list => Value::from_object(list_obj),
         pk => post.pk,
         body => &body_text,
         from => &envelope.field_from_to_string(),
@@ -300,6 +308,9 @@ pub async fn list_edit(
             url: ListEditPath(ListPathIdentifier::from(list.id.clone())).to_crumb(),
         },
     ];
+    let list_owners = db.list_owners(list.pk)?;
+    let mut list_obj = MailingList::from(list.clone());
+    list_obj.set_safety(list_owners.as_slice(), &state.conf.administrators);
     let context = minijinja::context! {
         canonical_url => ListEditPath(ListPathIdentifier::from(list.id.clone())).to_crumb(),
         page_title => format!("Edit {} settings", list.name),
@@ -310,7 +321,7 @@ pub async fn list_edit(
         post_count,
         subs_count,
         sub_requests_count,
-        list => Value::from_object(MailingList::from(list)),
+        list => Value::from_object(list_obj),
         current_user => auth.current_user,
         messages => session.drain_messages(),
         crumbs,
@@ -661,11 +672,14 @@ pub async fn list_subscribers(
             url: ListEditSubscribersPath(list.id.to_string().into()).to_crumb(),
         },
     ];
+    let list_owners = db.list_owners(list.pk)?;
+    let mut list_obj = MailingList::from(list.clone());
+    list_obj.set_safety(list_owners.as_slice(), &state.conf.administrators);
     let context = minijinja::context! {
         canonical_url => ListEditSubscribersPath(ListPathIdentifier::from(list.id.clone())).to_crumb(),
         page_title => format!("Subscribers of {}", list.name),
         subs,
-        list => Value::from_object(MailingList::from(list)),
+        list => Value::from_object(list_obj),
         current_user => auth.current_user,
         messages => session.drain_messages(),
         crumbs,
@@ -744,11 +758,13 @@ pub async fn list_candidates(
             url: ListEditCandidatesPath(list.id.to_string().into()).to_crumb(),
         },
     ];
+    let mut list_obj: MailingList = MailingList::from(list.clone());
+    list_obj.set_safety(list_owners.as_slice(), &state.conf.administrators);
     let context = minijinja::context! {
         canonical_url => ListEditCandidatesPath(ListPathIdentifier::from(list.id.clone())).to_crumb(),
         page_title => format!("Requests of {}", list.name),
         subs,
-        list => Value::from_object(MailingList::from(list)),
+        list => Value::from_object(list_obj),
         current_user => auth.current_user,
         messages => session.drain_messages(),
         crumbs,

web/src/main.rs

@@ -198,13 +198,14 @@ async fn root(
                     .earliest()
                     .map(|d| d.to_string())
             });
+            let list_owners = db.list_owners(list.pk)?;
+            let mut list_obj = MailingList::from(list.clone());
+            list_obj.set_safety(list_owners.as_slice(), &state.conf.administrators);
             Ok(minijinja::context! {
-                name => &list.name,
                 newest,
                 posts => &posts,
                 months => &months,
-                description => &list.description.as_deref().unwrap_or_default(),
-                list => Value::from_object(MailingList::from(list.clone())),
+                list => Value::from_object(list_obj),
             })
         })
         .collect::<Result<Vec<_>, mailpot::Error>>()?;

web/src/minijinja_utils.rs

@@ -21,6 +21,8 @@
 
 use std::fmt::Write;
 
+use mailpot::models::ListOwner;
+
 use super::*;
 
 mod compressed;
@@ -98,6 +100,29 @@ pub struct MailingList {
     #[serde(serialize_with = "super::utils::to_safe_string_opt")]
     pub archive_url: Option<String>,
     pub inner: DbVal<mailpot::models::MailingList>,
+    #[serde(default)]
+    pub is_description_html_safe: bool,
+}
+
+impl MailingList {
+    /// Set whether it's safe to not escape the list's description field.
+    ///
+    /// If anyone can display arbitrary html in the server, that's bad.
+    ///
+    /// Note: uses `Borrow` so that it can use both `DbVal<ListOwner>` and
+    /// `ListOwner` slices.
+    pub fn set_safety<O: std::borrow::Borrow<ListOwner>>(
+        &mut self,
+        owners: &[O],
+        administrators: &[String],
+    ) {
+        if owners.is_empty() || administrators.is_empty() {
+            return;
+        }
+        self.is_description_html_safe = owners
+            .iter()
+            .any(|o| administrators.contains(&o.borrow().address));
+    }
 }
 
 impl From<DbVal<mailpot::models::MailingList>> for MailingList {
@@ -124,6 +149,7 @@ impl From<DbVal<mailpot::models::MailingList>> for MailingList {
             topics,
             archive_url,
             inner: val,
+            is_description_html_safe: false,
         }
     }
 }
@@ -181,9 +207,18 @@ impl minijinja::value::StructObject for MailingList {
             "name" => Some(Value::from_serializable(&self.name)),
             "id" => Some(Value::from_serializable(&self.id)),
             "address" => Some(Value::from_serializable(&self.address)),
+            "description" if self.is_description_html_safe => {
+                self.description.as_ref().map_or_else(
+                    || Some(Value::from_serializable(&self.description)),
+                    |d| Some(Value::from_safe_string(d.clone())),
+                )
+            }
             "description" => Some(Value::from_serializable(&self.description)),
             "topics" => Some(Value::from_serializable(&self.topics)),
             "archive_url" => Some(Value::from_serializable(&self.archive_url)),
+            "is_description_html_safe" => {
+                Some(Value::from_serializable(&self.is_description_html_safe))
+            }
             _ => None,
         }
     }
@@ -198,6 +233,7 @@ impl minijinja::value::StructObject for MailingList {
                 "description",
                 "topics",
                 "archive_url",
+                "is_description_html_safe",
             ][..],
         )
     }
@@ -782,4 +818,46 @@ mod tests {
      0)(24, 0)(25, 0)(26, 0)(27, 0)(28, 0)(29, 0)(30, 0)"
 );
     }
+
+    #[test]
+    fn test_list_html_safe() {
+        let mut list = MailingList {
+            pk: 0,
+            name: String::new(),
+            id: String::new(),
+            address: String::new(),
+            description: None,
+            topics: vec![],
+            archive_url: None,
+            inner: DbVal(
+                mailpot::models::MailingList {
+                    pk: 0,
+                    name: String::new(),
+                    id: String::new(),
+                    address: String::new(),
+                    description: None,
+                    topics: vec![],
+                    archive_url: None,
+                },
+                0,
+            ),
+            is_description_html_safe: false,
+        };
+
+        let mut list_owners = vec![ListOwner {
+            pk: 0,
+            list: 0,
+            address: "admin@example.com".to_string(),
+            name: None,
+        }];
+        let administrators = vec!["admin@example.com".to_string()];
+        list.set_safety(&list_owners, &administrators);
+        assert!(list.is_description_html_safe);
+        list.set_safety::<ListOwner>(&[], &[]);
+        assert!(list.is_description_html_safe);
+        list.is_description_html_safe = false;
+        list_owners[0].address = "user@example.com".to_string();
+        list.set_safety(&list_owners, &administrators);
+        assert!(!list.is_description_html_safe);
+    }
 }

web/src/settings.rs

@@ -302,6 +302,9 @@ pub async fn user_list_subscription(
         },
     ];
 
+    let list_owners = db.list_owners(list.pk)?;
+    let mut list = crate::minijinja_utils::MailingList::from(list);
+    list.set_safety(list_owners.as_slice(), &state.conf.administrators);
     let context = minijinja::context! {
         page_title => "Subscription settings",
         user => user,

web/src/templates/lists.html

@@ -5,7 +5,7 @@
         <dl class="lists" aria-label="list of mailing lists">
             {% for l in lists %}
                 <dt aria-label="mailing list name"><a href="{{ list_path(l.list.id) }}">{{ l.list.name }}</a></dt>
-                <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>
+                <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>
             {% endfor %}
         </dl>
     </div>

web/src/templates/lists/edit.html

@@ -5,7 +5,11 @@
         {{ list.name }} <a href="mailto:{{ list.address | safe }}"><code>{{ list.address }}</code></a>
     </address>
     {% if list.description %}
-        <p>{{ list.description }}</p>
+        {% if list.is_description_html_safe %}
+            {{ list.description|safe}}
+        {% else %}
+            <p>{{ list.description }}</p>
+        {% endif %}
     {% endif %}
     {% if list.archive_url %}
         <p><a href="{{ list.archive_url }}">{{ list.archive_url }}</a></p>

web/src/templates/lists/list.html

@@ -5,7 +5,7 @@
         <br aria-hidden="true">
     {% endif %}
     {% if list.description %}
-        <p title="mailing list description">List description: {{ list.description }}</p>
+        <p title="mailing list description">{{ list.description }}</p>
     {% else %}
         <p title="mailing list description">No list description.</p>
     {% endif %}
@@ -17,12 +17,14 @@
                 <input type="hidden" name="list_pk", value="{{ list.pk }}">
                 <input type="submit" name="unsubscribe" value="Unsubscribe as {{ current_user.address }}">
             </form>
+            <br />
         {% else %}
             <form method="post" action="{{ settings_path() }}" class="settings-form">
                 <input type="hidden" name="type", value="subscribe">
                 <input type="hidden" name="list_pk", value="{{ list.pk }}">
                 <input type="submit" name="subscribe" value="Subscribe as {{ current_user.address }}">
             </form>
+            <br />
         {% endif %}
     {% endif %}
     {% if preamble %}

web/src/templates/settings_subscription.html

@@ -4,7 +4,9 @@
     <address>
         {{ list.name }} <a href="mailto:{{ list.address | safe }}"><code>{{ list.address }}</code></a>
     </address>
-    {% if list.description %}
+    {% if list.is_description_html_safe %}
+        {{ list.description|safe}}
+    {% else %}
         <p>{{ list.description }}</p>
     {% endif %}
     {% if list.archive_url %}