From 925e644d54fe4cbb54446aee1841f6706ca3f034 Mon Sep 17 00:00:00 2001 From: lovasoa Date: Wed, 26 Feb 2025 11:13:27 +0100 Subject: [PATCH 01/11] new crate feature: allow_all_protocols_in_img test "unsafe" protocols on image sources here: https://jsfiddle.net/#&togetherjs=Q4ctsPpdfN --- Cargo.toml | 1 + readme.md | 4 ++++ src/configuration.rs | 3 +++ src/to_html.rs | 6 +++++- src/util/constant.rs | 5 +++++ tests/image.rs | 17 ++++++++++++++++ tests/misc_dangerous_protocol.rs | 35 ++++++++++++++++++++++++++++++++ 7 files changed, 70 insertions(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index bd3854ba..62d5bed4 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -25,6 +25,7 @@ default = [] json = ["serde"] log = ["dep:log"] serde = ["dep:serde"] +allow_all_protocols_in_img = [] [package] authors = ["Titus Wormer "] diff --git a/readme.md b/readme.md index 7f26efc7..08394da3 100644 --- a/readme.md +++ b/readme.md @@ -290,6 +290,10 @@ protocols in links/images (such as `javascript:` or `data:`). dangerous protocols are used, as it encodes or drops them. Turning on the `allow_dangerous_html` or `allow_dangerous_protocol` options for user-provided markdown opens you up to XSS attacks. + - `allow_dangerous_html` allows HTML tags to be rendered, including tags that may trigger the execution of scripts + - `allow_dangerous_protocol` allows the use of protocols like `javascript:`, in links/images + - when the crate feature `allow_all_protocols_in_img` is enabled, `allow_dangerous_protocol` will only apply to links. + - The [HTML specification](https://html.spec.whatwg.org/multipage/images.html#images-processing-model) does not allow the execution of scripts in images, whatever the protocol they use. All modern browsers respect this. An aspect related to XSS for security is syntax errors: markdown itself has no syntax errors. diff --git a/src/configuration.rs b/src/configuration.rs index 083f8dba..e4e60cf4 100644 --- a/src/configuration.rs +++ b/src/configuration.rs @@ -522,6 +522,9 @@ pub struct CompileOptions { /// `ircs`, `mailto`, `xmpp`), are safe. /// All other URLs are dangerous and dropped. /// + /// When the crate feature `allow_all_protocols_in_img` is enabled, `allow_dangerous_protocol` will only apply to links. + /// The [HTML specification](https://html.spec.whatwg.org/multipage/images.html#images-processing-model) does not allow the execution of scripts in images, whatever the protocol they use. All modern browsers respect this. + /// /// ## Examples /// /// ``` diff --git a/src/to_html.rs b/src/to_html.rs index dbb560dc..46ced7b4 100644 --- a/src/to_html.rs +++ b/src/to_html.rs @@ -1,6 +1,7 @@ //! Turn events into a string of HTML. use crate::event::{Event, Kind, Name}; use crate::mdast::AlignKind; +use crate::util::constant::ALLOW_ALL_PROTOCOLS_IN_IMG; use crate::util::{ character_reference::decode as decode_character_reference, constant::{SAFE_PROTOCOL_HREF, SAFE_PROTOCOL_SRC}, @@ -1457,7 +1458,10 @@ fn on_exit_media(context: &mut CompileContext) { }; if let Some(destination) = destination { - let url = if context.options.allow_dangerous_protocol { + let allow_dangerous_protocol = context.options.allow_dangerous_protocol + || (ALLOW_ALL_PROTOCOLS_IN_IMG && media.image); + + let url = if allow_dangerous_protocol { sanitize(destination) } else { sanitize_with_protocols( diff --git a/src/util/constant.rs b/src/util/constant.rs index cf27f53e..183087f1 100644 --- a/src/util/constant.rs +++ b/src/util/constant.rs @@ -276,6 +276,11 @@ pub const SAFE_PROTOCOL_HREF: [&str; 6] = ["http", "https", "irc", "ircs", "mail /// This list is based on what is allowed by GitHub. pub const SAFE_PROTOCOL_SRC: [&str; 2] = ["http", "https"]; +#[cfg(feature = "allow_all_protocols_in_img")] +pub const ALLOW_ALL_PROTOCOLS_IN_IMG: bool = true; +#[cfg(not(feature = "allow_all_protocols_in_img"))] +pub const ALLOW_ALL_PROTOCOLS_IN_IMG: bool = false; + /// The number of characters that form a tab stop. /// /// This relates to the number of whitespace characters needed to form certain diff --git a/tests/image.rs b/tests/image.rs index 19144d29..76f9808b 100644 --- a/tests/image.rs +++ b/tests/image.rs @@ -214,6 +214,7 @@ fn image() -> Result<(), message::Message> { "should support turning off label start (image)" ); + #[cfg(not(feature = "allow_all_protocols_in_img"))] assert_eq!( to_html("![](javascript:alert(1))"), "

\"\"

", @@ -235,6 +236,22 @@ fn image() -> Result<(), message::Message> { "should allow non-http protocols w/ `allowDangerousProtocol`" ); + #[cfg(feature = "allow_all_protocols_in_img")] + assert_eq!( + to_html_with_options( + "![](javascript:alert(1))", + &Options { + compile: CompileOptions { + allow_dangerous_protocol: false, + ..Default::default() + }, + ..Default::default() + } + )?, + "

\"\"

", + "should allow non-http protocols w/ `allowDangerousProtocol` when allow_all_protocols_in_img feature is enabled" + ); + assert_eq!( to_mdast( "a ![alpha]() b ![bravo](charlie 'delta') c.", diff --git a/tests/misc_dangerous_protocol.rs b/tests/misc_dangerous_protocol.rs index c0550547..c1800c6f 100644 --- a/tests/misc_dangerous_protocol.rs +++ b/tests/misc_dangerous_protocol.rs @@ -36,6 +36,7 @@ fn dangerous_protocol_autolink() { #[test] fn dangerous_protocol_image() { + #[cfg(not(feature = "allow_all_protocols_in_img"))] assert_eq!( to_html("![](javascript:alert(1))"), "

\"\"

", @@ -54,12 +55,14 @@ fn dangerous_protocol_image() { "should allow `https:`" ); + #[cfg(not(feature = "allow_all_protocols_in_img"))] assert_eq!( to_html("![](irc:///help)"), "

\"\"

", "should not allow `irc:`" ); + #[cfg(not(feature = "allow_all_protocols_in_img"))] assert_eq!( to_html("![](mailto:a)"), "

\"\"

", @@ -195,3 +198,35 @@ fn dangerous_protocol_link() { "should allow a colon in a path" ); } + +#[cfg(feature = "allow_all_protocols_in_img")] +#[test] +fn dangerous_protocol_image_with_feature() { + use markdown::{to_html_with_options, CompileOptions, Options}; + + let options = Options { + compile: CompileOptions { + allow_dangerous_protocol: true, + ..Default::default() + }, + ..Default::default() + }; + + let result = to_html_with_options("![](javascript:alert(1))", &options).unwrap(); + assert_eq!( + result, "

\"\"

", + "should allow javascript protocol with allow_all_protocols_in_img feature" + ); + + let result = to_html_with_options("![](irc:///help)", &options).unwrap(); + assert_eq!( + result, "

\"\"

", + "should allow irc protocol with allow_all_protocols_in_img feature" + ); + + let result = to_html_with_options("![](mailto:a)", &options).unwrap(); + assert_eq!( + result, "

\"\"

", + "should allow mailto protocol with allow_all_protocols_in_img feature" + ); +} From ee8ee437db1e15c9cf583c2fb30638f0c066213e Mon Sep 17 00:00:00 2001 From: lovasoa Date: Thu, 27 Feb 2025 11:29:21 +0100 Subject: [PATCH 02/11] use a config option instead of a crate feature --- Cargo.toml | 1 - readme.md | 10 ++++++-- src/configuration.rs | 42 ++++++++++++++++++++++++++++++++ src/to_html.rs | 3 +-- src/util/constant.rs | 5 ---- tests/image.rs | 5 ++-- tests/misc_dangerous_protocol.rs | 14 ++++------- 7 files changed, 58 insertions(+), 22 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 62d5bed4..bd3854ba 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -25,7 +25,6 @@ default = [] json = ["serde"] log = ["dep:log"] serde = ["dep:serde"] -allow_all_protocols_in_img = [] [package] authors = ["Titus Wormer "] diff --git a/readme.md b/readme.md index 08394da3..9dcb5ecd 100644 --- a/readme.md +++ b/readme.md @@ -285,14 +285,20 @@ The following bash scripts are useful when working on this project: The typical security aspect discussed for markdown is [cross-site scripting (XSS)][xss] attacks. Markdown itself is safe if it does not include embedded HTML or dangerous -protocols in links/images (such as `javascript:` or `data:`). +protocols in links/images (such as `javascript:`). `markdown-rs` makes any markdown safe by default, even if HTML is embedded or dangerous protocols are used, as it encodes or drops them. + +Some very old browsers (such as Opera 12 from 2012) did not respect the HTML +specification and executed scripts in images, making the use of external images +dangerous. However, all modern browsers respect the HTML specification and +prevent this, making the use of external images safe. + Turning on the `allow_dangerous_html` or `allow_dangerous_protocol` options for user-provided markdown opens you up to XSS attacks. - `allow_dangerous_html` allows HTML tags to be rendered, including tags that may trigger the execution of scripts - `allow_dangerous_protocol` allows the use of protocols like `javascript:`, in links/images - - when the crate feature `allow_all_protocols_in_img` is enabled, `allow_dangerous_protocol` will only apply to links. + - when the option `allow_any_img_src` is enabled, `allow_dangerous_protocol` will only apply to links. - The [HTML specification](https://html.spec.whatwg.org/multipage/images.html#images-processing-model) does not allow the execution of scripts in images, whatever the protocol they use. All modern browsers respect this. An aspect related to XSS for security is syntax errors: markdown itself has no diff --git a/src/configuration.rs b/src/configuration.rs index e4e60cf4..5dc6387b 100644 --- a/src/configuration.rs +++ b/src/configuration.rs @@ -556,6 +556,48 @@ pub struct CompileOptions { /// ``` pub allow_dangerous_protocol: bool, + /// Whether to allow all protocols in images. + /// + /// The default is `false`, which means `allow_dangerous_protocol` controls protocol safety for both links and images. + /// + /// Pass `true` to allow all protocols in images, regardless of the `allow_dangerous_protocol` setting. + /// This is safe because the [HTML specification](https://html.spec.whatwg.org/multipage/images.html#images-processing-model) + /// does not allow the execution of scripts in images, whatever the protocol they use. + /// + /// ## Examples + /// + /// ``` + /// use markdown::{to_html_with_options, CompileOptions, Options}; + /// # fn main() -> Result<(), markdown::message::Message> { + /// + /// // By default, some protocols in image sources are dropped: + /// assert_eq!( + /// to_html_with_options( + /// "![](data:image/gif;base64,R0lGODlhAQABAAAAACH5BAEKAAEALAAAAAABAAEAAAICTAEAOw==)", + /// &Options::default() + /// )?, + /// "

\"\"

" + /// ); + /// + /// // Turn `allow_any_img_src` on to allow all protocols in images: + /// assert_eq!( + /// to_html_with_options( + /// "![](javascript:alert(1))", + /// &Options { + /// compile: CompileOptions { + /// allow_any_img_src: true, + /// ..CompileOptions::default() + /// }, + /// ..Options::default() + /// } + /// )?, // This is safe because browsers do not execute scripts in image sources. + /// "

\"\"

" + /// ); + /// # Ok(()) + /// # } + /// ``` + pub allow_any_img_src: bool, + // To do: `doc_markdown` is broken. #[allow(clippy::doc_markdown)] /// Default line ending to use when compiling to HTML, for line endings not diff --git a/src/to_html.rs b/src/to_html.rs index 46ced7b4..af4f8e7f 100644 --- a/src/to_html.rs +++ b/src/to_html.rs @@ -1,7 +1,6 @@ //! Turn events into a string of HTML. use crate::event::{Event, Kind, Name}; use crate::mdast::AlignKind; -use crate::util::constant::ALLOW_ALL_PROTOCOLS_IN_IMG; use crate::util::{ character_reference::decode as decode_character_reference, constant::{SAFE_PROTOCOL_HREF, SAFE_PROTOCOL_SRC}, @@ -1459,7 +1458,7 @@ fn on_exit_media(context: &mut CompileContext) { if let Some(destination) = destination { let allow_dangerous_protocol = context.options.allow_dangerous_protocol - || (ALLOW_ALL_PROTOCOLS_IN_IMG && media.image); + || (context.options.allow_any_img_src && media.image); let url = if allow_dangerous_protocol { sanitize(destination) diff --git a/src/util/constant.rs b/src/util/constant.rs index 183087f1..cf27f53e 100644 --- a/src/util/constant.rs +++ b/src/util/constant.rs @@ -276,11 +276,6 @@ pub const SAFE_PROTOCOL_HREF: [&str; 6] = ["http", "https", "irc", "ircs", "mail /// This list is based on what is allowed by GitHub. pub const SAFE_PROTOCOL_SRC: [&str; 2] = ["http", "https"]; -#[cfg(feature = "allow_all_protocols_in_img")] -pub const ALLOW_ALL_PROTOCOLS_IN_IMG: bool = true; -#[cfg(not(feature = "allow_all_protocols_in_img"))] -pub const ALLOW_ALL_PROTOCOLS_IN_IMG: bool = false; - /// The number of characters that form a tab stop. /// /// This relates to the number of whitespace characters needed to form certain diff --git a/tests/image.rs b/tests/image.rs index 76f9808b..2827daff 100644 --- a/tests/image.rs +++ b/tests/image.rs @@ -214,7 +214,6 @@ fn image() -> Result<(), message::Message> { "should support turning off label start (image)" ); - #[cfg(not(feature = "allow_all_protocols_in_img"))] assert_eq!( to_html("![](javascript:alert(1))"), "

\"\"

", @@ -236,20 +235,20 @@ fn image() -> Result<(), message::Message> { "should allow non-http protocols w/ `allowDangerousProtocol`" ); - #[cfg(feature = "allow_all_protocols_in_img")] assert_eq!( to_html_with_options( "![](javascript:alert(1))", &Options { compile: CompileOptions { allow_dangerous_protocol: false, + allow_any_img_src: true, ..Default::default() }, ..Default::default() } )?, "

\"\"

", - "should allow non-http protocols w/ `allowDangerousProtocol` when allow_all_protocols_in_img feature is enabled" + "should allow non-http protocols with the `allow_any_img_src` option" ); assert_eq!( diff --git a/tests/misc_dangerous_protocol.rs b/tests/misc_dangerous_protocol.rs index c1800c6f..733d936f 100644 --- a/tests/misc_dangerous_protocol.rs +++ b/tests/misc_dangerous_protocol.rs @@ -36,7 +36,6 @@ fn dangerous_protocol_autolink() { #[test] fn dangerous_protocol_image() { - #[cfg(not(feature = "allow_all_protocols_in_img"))] assert_eq!( to_html("![](javascript:alert(1))"), "

\"\"

", @@ -55,14 +54,12 @@ fn dangerous_protocol_image() { "should allow `https:`" ); - #[cfg(not(feature = "allow_all_protocols_in_img"))] assert_eq!( to_html("![](irc:///help)"), "

\"\"

", "should not allow `irc:`" ); - #[cfg(not(feature = "allow_all_protocols_in_img"))] assert_eq!( to_html("![](mailto:a)"), "

\"\"

", @@ -199,14 +196,13 @@ fn dangerous_protocol_link() { ); } -#[cfg(feature = "allow_all_protocols_in_img")] #[test] -fn dangerous_protocol_image_with_feature() { +fn dangerous_protocol_image_with_option() { use markdown::{to_html_with_options, CompileOptions, Options}; let options = Options { compile: CompileOptions { - allow_dangerous_protocol: true, + allow_any_img_src: true, ..Default::default() }, ..Default::default() @@ -215,18 +211,18 @@ fn dangerous_protocol_image_with_feature() { let result = to_html_with_options("![](javascript:alert(1))", &options).unwrap(); assert_eq!( result, "

\"\"

", - "should allow javascript protocol with allow_all_protocols_in_img feature" + "should allow javascript protocol with allow_any_img_src option" ); let result = to_html_with_options("![](irc:///help)", &options).unwrap(); assert_eq!( result, "

\"\"

", - "should allow irc protocol with allow_all_protocols_in_img feature" + "should allow irc protocol with allow_any_img_src option" ); let result = to_html_with_options("![](mailto:a)", &options).unwrap(); assert_eq!( result, "

\"\"

", - "should allow mailto protocol with allow_all_protocols_in_img feature" + "should allow mailto protocol with allow_any_img_src option" ); } From aead95b40feb95d1a71bf738954bd6c53eddfd62 Mon Sep 17 00:00:00 2001 From: lovasoa Date: Thu, 27 Feb 2025 11:40:31 +0100 Subject: [PATCH 03/11] better wording of readme --- readme.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/readme.md b/readme.md index 9dcb5ecd..3eeccfb1 100644 --- a/readme.md +++ b/readme.md @@ -290,8 +290,9 @@ protocols in links/images (such as `javascript:`). dangerous protocols are used, as it encodes or drops them. Some very old browsers (such as Opera 12 from 2012) did not respect the HTML -specification and executed scripts in images, making the use of external images -dangerous. However, all modern browsers respect the HTML specification and +specification and executed scripts in images, rendering sites with user-controlled +images vulnerable to XSS attacks. +However, all modern browsers respect the HTML specification and prevent this, making the use of external images safe. Turning on the `allow_dangerous_html` or `allow_dangerous_protocol` options for From 48adc6974ea0bf0092b08271beeadbd89dd6f0d6 Mon Sep 17 00:00:00 2001 From: Ophir LOJKINE Date: Thu, 27 Feb 2025 13:48:58 +0100 Subject: [PATCH 04/11] Update src/configuration.rs Co-authored-by: Titus Signed-off-by: Ophir LOJKINE --- src/configuration.rs | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/src/configuration.rs b/src/configuration.rs index 5dc6387b..029e085b 100644 --- a/src/configuration.rs +++ b/src/configuration.rs @@ -556,13 +556,19 @@ pub struct CompileOptions { /// ``` pub allow_dangerous_protocol: bool, - /// Whether to allow all protocols in images. - /// - /// The default is `false`, which means `allow_dangerous_protocol` controls protocol safety for both links and images. - /// - /// Pass `true` to allow all protocols in images, regardless of the `allow_dangerous_protocol` setting. - /// This is safe because the [HTML specification](https://html.spec.whatwg.org/multipage/images.html#images-processing-model) - /// does not allow the execution of scripts in images, whatever the protocol they use. + /// Whether to allow all values in images. + /// + /// The default is `false`, + /// which lets `allow_dangerous_protocol` control protocol safety for + /// both links and images. + /// + /// Pass `true` to allow all values as `src` on images, + /// regardless of `allow_dangerous_protocol`. + /// This is safe because the + /// [HTML specification][whatwg-html-image-processing] + /// does not allow executable code in images. + /// + /// [whatwg-html-image-processing]: https://html.spec.whatwg.org/multipage/images.html#images-processing-model /// /// ## Examples /// From 24765be929c6cb1b0316ff0cbbfd2dc8157089f9 Mon Sep 17 00:00:00 2001 From: Ophir LOJKINE Date: Thu, 27 Feb 2025 13:49:29 +0100 Subject: [PATCH 05/11] Update src/configuration.rs Co-authored-by: Titus Signed-off-by: Ophir LOJKINE --- src/configuration.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/configuration.rs b/src/configuration.rs index 029e085b..ae4c1015 100644 --- a/src/configuration.rs +++ b/src/configuration.rs @@ -585,7 +585,8 @@ pub struct CompileOptions { /// "

\"\"

" /// ); /// - /// // Turn `allow_any_img_src` on to allow all protocols in images: + /// // Turn `allow_any_img_src` on to allow all values as `src` on images. + /// // This is safe because browsers do not execute code in images. /// assert_eq!( /// to_html_with_options( /// "![](javascript:alert(1))", From e9eb01afb99c938dbfb4eda6fd19cb29dbde3a85 Mon Sep 17 00:00:00 2001 From: Ophir LOJKINE Date: Thu, 27 Feb 2025 13:49:48 +0100 Subject: [PATCH 06/11] Update src/configuration.rs Co-authored-by: Titus Signed-off-by: Ophir LOJKINE --- src/configuration.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/configuration.rs b/src/configuration.rs index ae4c1015..64ca6ff3 100644 --- a/src/configuration.rs +++ b/src/configuration.rs @@ -597,7 +597,7 @@ pub struct CompileOptions { /// }, /// ..Options::default() /// } - /// )?, // This is safe because browsers do not execute scripts in image sources. + /// )?, /// "

\"\"

" /// ); /// # Ok(()) From 9fd503e1786738cf9de15ad9e2af65c0f68bf197 Mon Sep 17 00:00:00 2001 From: Ophir LOJKINE Date: Thu, 27 Feb 2025 13:49:59 +0100 Subject: [PATCH 07/11] Update src/configuration.rs Co-authored-by: Titus Signed-off-by: Ophir LOJKINE --- src/configuration.rs | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/configuration.rs b/src/configuration.rs index 64ca6ff3..75917788 100644 --- a/src/configuration.rs +++ b/src/configuration.rs @@ -522,8 +522,15 @@ pub struct CompileOptions { /// `ircs`, `mailto`, `xmpp`), are safe. /// All other URLs are dangerous and dropped. /// - /// When the crate feature `allow_all_protocols_in_img` is enabled, `allow_dangerous_protocol` will only apply to links. - /// The [HTML specification](https://html.spec.whatwg.org/multipage/images.html#images-processing-model) does not allow the execution of scripts in images, whatever the protocol they use. All modern browsers respect this. + /// When the option `allow_all_protocols_in_img` is enabled, + /// `allow_dangerous_protocol` only applies to links. + /// + /// This is safe because the + /// [HTML specification][whatwg-html-image-processing] + /// does not allow executable code in images. + /// All modern browsers respect this. + /// + /// [whatwg-html-image-processing]: https://html.spec.whatwg.org/multipage/images.html#images-processing-model /// /// ## Examples /// From c02396fc126be896a033d5f21c22c05ba10f84f9 Mon Sep 17 00:00:00 2001 From: lovasoa Date: Thu, 27 Feb 2025 13:51:16 +0100 Subject: [PATCH 08/11] remove options explanation from readme --- readme.md | 4 ---- 1 file changed, 4 deletions(-) diff --git a/readme.md b/readme.md index 3eeccfb1..e6e05bbc 100644 --- a/readme.md +++ b/readme.md @@ -297,10 +297,6 @@ prevent this, making the use of external images safe. Turning on the `allow_dangerous_html` or `allow_dangerous_protocol` options for user-provided markdown opens you up to XSS attacks. - - `allow_dangerous_html` allows HTML tags to be rendered, including tags that may trigger the execution of scripts - - `allow_dangerous_protocol` allows the use of protocols like `javascript:`, in links/images - - when the option `allow_any_img_src` is enabled, `allow_dangerous_protocol` will only apply to links. - - The [HTML specification](https://html.spec.whatwg.org/multipage/images.html#images-processing-model) does not allow the execution of scripts in images, whatever the protocol they use. All modern browsers respect this. An aspect related to XSS for security is syntax errors: markdown itself has no syntax errors. From 9e85bf9f71332213dc49882a3983e295ee8dd20e Mon Sep 17 00:00:00 2001 From: lovasoa Date: Thu, 27 Feb 2025 13:55:37 +0100 Subject: [PATCH 09/11] warning wording --- readme.md | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/readme.md b/readme.md index e6e05bbc..873335ec 100644 --- a/readme.md +++ b/readme.md @@ -289,11 +289,13 @@ protocols in links/images (such as `javascript:`). `markdown-rs` makes any markdown safe by default, even if HTML is embedded or dangerous protocols are used, as it encodes or drops them. -Some very old browsers (such as Opera 12 from 2012) did not respect the HTML -specification and executed scripts in images, rendering sites with user-controlled -images vulnerable to XSS attacks. -However, all modern browsers respect the HTML specification and -prevent this, making the use of external images safe. +You should be able to set `allow_any_src` safely. +The default is to allow only `http:`, `https:`, and relative images, +which is what GitHub does. +But it should be safe to allow any value on `src`. +The [HTML specification][whatwg-html-image] prohibits dangerous scripts in +images and all modern browsers respect this and are thus safe. +Opera 12 (from 2012) is a notable browser that did not respect this. Turning on the `allow_dangerous_html` or `allow_dangerous_protocol` options for user-provided markdown opens you up to XSS attacks. @@ -420,3 +422,5 @@ Special thanks go out to: [support]: .github/support.md [coc]: .github/code-of-conduct.md + +[whatwg-html-image]: https://html.spec.whatwg.org/multipage/images.html#images-processing-model \ No newline at end of file From 18b93b0e7c4b12de0a71f8f419c016b23e4e8539 Mon Sep 17 00:00:00 2001 From: lovasoa Date: Thu, 27 Feb 2025 13:57:47 +0100 Subject: [PATCH 10/11] move warning --- readme.md | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/readme.md b/readme.md index 873335ec..1f1e88f0 100644 --- a/readme.md +++ b/readme.md @@ -289,17 +289,17 @@ protocols in links/images (such as `javascript:`). `markdown-rs` makes any markdown safe by default, even if HTML is embedded or dangerous protocols are used, as it encodes or drops them. -You should be able to set `allow_any_src` safely. +Turning on the `allow_dangerous_html` or `allow_dangerous_protocol` options for +user-provided markdown opens you up to XSS attacks. + +Additionnally, you should be able to set `allow_any_img_src` safely. The default is to allow only `http:`, `https:`, and relative images, -which is what GitHub does. -But it should be safe to allow any value on `src`. +which is what GitHub does. But it should be safe to allow any value on `src`. + The [HTML specification][whatwg-html-image] prohibits dangerous scripts in images and all modern browsers respect this and are thus safe. Opera 12 (from 2012) is a notable browser that did not respect this. -Turning on the `allow_dangerous_html` or `allow_dangerous_protocol` options for -user-provided markdown opens you up to XSS attacks. - An aspect related to XSS for security is syntax errors: markdown itself has no syntax errors. Some syntax extensions (specifically, only MDX) do include syntax errors. From 519a4924a05c9ceb1626c490262903eda80129f8 Mon Sep 17 00:00:00 2001 From: lovasoa Date: Thu, 27 Feb 2025 13:58:25 +0100 Subject: [PATCH 11/11] fmt --- src/configuration.rs | 44 ++++++++++++++++++++++---------------------- 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/src/configuration.rs b/src/configuration.rs index 75917788..9d97351b 100644 --- a/src/configuration.rs +++ b/src/configuration.rs @@ -522,15 +522,15 @@ pub struct CompileOptions { /// `ircs`, `mailto`, `xmpp`), are safe. /// All other URLs are dangerous and dropped. /// - /// When the option `allow_all_protocols_in_img` is enabled, - /// `allow_dangerous_protocol` only applies to links. - /// - /// This is safe because the - /// [HTML specification][whatwg-html-image-processing] - /// does not allow executable code in images. - /// All modern browsers respect this. - /// - /// [whatwg-html-image-processing]: https://html.spec.whatwg.org/multipage/images.html#images-processing-model + /// When the option `allow_all_protocols_in_img` is enabled, + /// `allow_dangerous_protocol` only applies to links. + /// + /// This is safe because the + /// [HTML specification][whatwg-html-image-processing] + /// does not allow executable code in images. + /// All modern browsers respect this. + /// + /// [whatwg-html-image-processing]: https://html.spec.whatwg.org/multipage/images.html#images-processing-model /// /// ## Examples /// @@ -563,19 +563,19 @@ pub struct CompileOptions { /// ``` pub allow_dangerous_protocol: bool, - /// Whether to allow all values in images. - /// - /// The default is `false`, - /// which lets `allow_dangerous_protocol` control protocol safety for - /// both links and images. - /// - /// Pass `true` to allow all values as `src` on images, - /// regardless of `allow_dangerous_protocol`. - /// This is safe because the - /// [HTML specification][whatwg-html-image-processing] - /// does not allow executable code in images. - /// - /// [whatwg-html-image-processing]: https://html.spec.whatwg.org/multipage/images.html#images-processing-model + /// Whether to allow all values in images. + /// + /// The default is `false`, + /// which lets `allow_dangerous_protocol` control protocol safety for + /// both links and images. + /// + /// Pass `true` to allow all values as `src` on images, + /// regardless of `allow_dangerous_protocol`. + /// This is safe because the + /// [HTML specification][whatwg-html-image-processing] + /// does not allow executable code in images. + /// + /// [whatwg-html-image-processing]: https://html.spec.whatwg.org/multipage/images.html#images-processing-model /// /// ## Examples ///