Fix XML CDATA injection (credits to @andreymal for the report)

This commit is contained in:
mdecimus 2025-05-29 19:59:51 +02:00
parent 207d370fd9
commit fe24cbf41d
3 changed files with 70 additions and 25 deletions

View file

@ -5,7 +5,6 @@
*/
use std::{
collections::BTreeMap,
net::{IpAddr, Ipv4Addr},
path::PathBuf,
sync::Arc,

View file

@ -25,6 +25,10 @@ trait XmlEscape {
fn write_escaped_to(&self, out: &mut impl Write) -> std::fmt::Result;
}
trait XmlCdataEscape {
fn write_cdata_escaped_to(&self, out: &mut impl Write) -> std::fmt::Result;
}
impl<T: AsRef<str>> XmlEscape for T {
fn write_escaped_to(&self, out: &mut impl Write) -> std::fmt::Result {
let str = self.as_ref();
@ -44,6 +48,30 @@ impl<T: AsRef<str>> XmlEscape for T {
}
}
impl<T: AsRef<str>> XmlCdataEscape for T {
fn write_cdata_escaped_to(&self, out: &mut impl Write) -> std::fmt::Result {
let str = self.as_ref();
let mut last_ch = '\0';
let mut last_ch2 = '\0';
out.write_str("<![CDATA[")?;
for ch in str.chars() {
match ch {
'>' if last_ch == ']' && last_ch2 == ']' => {
out.write_str("]]><![CDATA[>")?;
}
_ => out.write_char(ch)?,
}
last_ch2 = last_ch;
last_ch = ch;
}
out.write_str("]]>")
}
}
impl Display for Namespaces {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
f.write_str("xmlns:D=\"DAV:\"")?;
@ -176,6 +204,7 @@ mod tests {
use crate::{
parser::{tokenizer::Tokenizer, Token},
responses::XmlCdataEscape,
schema::{
property::{
ActiveLock, CalDavProperty, CardDavProperty, DavValue, LockScope, Privilege,
@ -199,6 +228,18 @@ mod tests {
}
}
impl From<ICalendar> for DavValue {
fn from(v: ICalendar) -> Self {
DavValue::ICalendar(v)
}
}
impl From<VCard> for DavValue {
fn from(v: VCard) -> Self {
DavValue::VCard(v)
}
}
#[test]
fn parse_responses() {
for (num, test) in [
@ -634,4 +675,30 @@ END:VCARD
}
}
}
#[test]
fn escape_cdata() {
for (test, expected) in [
("", "<![CDATA[]]>"),
("hello", "<![CDATA[hello]]>"),
("hello world", "<![CDATA[hello world]]>"),
("<hello>", "<![CDATA[<hello>]]>"),
("&hello;", "<![CDATA[&hello;]]>"),
("'hello'", "<![CDATA['hello']]>"),
("\"hello\"", "<![CDATA[\"hello\"]]>"),
("<>&'\"", "<![CDATA[<>&'\"]]>"),
(">", "<![CDATA[>]]>"),
("]]>]", "<![CDATA[]]]]><![CDATA[>]]]>"),
("]]>", "<![CDATA[]]]]><![CDATA[>]]>"),
("hello]]>world", "<![CDATA[hello]]]]><![CDATA[>world]]>"),
(
"hello]]><nasty-xml>pure-evil</nasty-xml>",
"<![CDATA[hello]]]]><![CDATA[><nasty-xml>pure-evil</nasty-xml>]]>",
),
] {
let mut output = String::new();
test.write_cdata_escaped_to(&mut output).unwrap();
assert_eq!(output, expected, "failed for input: {test:?}");
}
}
}

View file

@ -4,7 +4,7 @@
* SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-SEL
*/
use super::XmlEscape;
use super::{XmlCdataEscape, XmlEscape};
use crate::schema::{
property::{
ActiveLock, CalDavProperty, CardDavProperty, Comp, DavProperty, DavValue, LockDiscovery,
@ -15,7 +15,6 @@ use crate::schema::{
response::{Ace, AclRestrictions, Href, List, PropResponse, SupportedPrivilege},
Namespace, Namespaces,
};
use calcard::{icalendar::ICalendar, vcard::VCard};
use mail_parser::{
parsers::fields::date::{DOW, MONTH},
DateTime,
@ -88,15 +87,7 @@ impl Display for DavValue {
DavValue::ActiveLocks(v) => v.fmt(f),
DavValue::LockEntries(v) => v.fmt(f),
DavValue::ReportSets(v) => v.fmt(f),
DavValue::VCard(v) => {
write!(f, "<![CDATA[{v}]]>")
}
DavValue::ICalendar(v) => {
write!(f, "<![CDATA[{v}]]>")
}
DavValue::CData(v) => {
write!(f, "<![CDATA[{v}]]>")
}
DavValue::CData(v) => v.write_cdata_escaped_to(f),
DavValue::Components(v) => v.fmt(f),
DavValue::Collations(v) => v.fmt(f),
DavValue::Href(v) => v.fmt(f),
@ -145,7 +136,7 @@ impl Display for DavValue {
)
}
DavValue::Response(v) => v.fmt(f),
DavValue::Null => Ok(()),
DavValue::VCard(_) | DavValue::ICalendar(_) | DavValue::Null => Ok(()),
}
}
}
@ -353,18 +344,6 @@ impl From<Vec<SupportedCollation>> for DavValue {
}
}
impl From<ICalendar> for DavValue {
fn from(v: ICalendar) -> Self {
DavValue::ICalendar(v)
}
}
impl From<VCard> for DavValue {
fn from(v: VCard) -> Self {
DavValue::VCard(v)
}
}
impl From<SupportedLock> for DavValue {
fn from(v: SupportedLock) -> Self {
DavValue::LockEntries(v.0)