diff --git a/readme.md b/readme.md index 7f26efc7..1f1e88f0 100644 --- a/readme.md +++ b/readme.md @@ -285,12 +285,21 @@ 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. + 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`. + +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. + 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. @@ -413,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 diff --git a/src/configuration.rs b/src/configuration.rs index 083f8dba..9d97351b 100644 --- a/src/configuration.rs +++ b/src/configuration.rs @@ -522,6 +522,16 @@ 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 + /// /// ## Examples /// /// ``` @@ -553,6 +563,55 @@ 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 + /// + /// ## 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 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))", + /// &Options { + /// compile: CompileOptions { + /// allow_any_img_src: true, + /// ..CompileOptions::default() + /// }, + /// ..Options::default() + /// } + /// )?, + /// "

\"\"

" + /// ); + /// # 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 dbb560dc..af4f8e7f 100644 --- a/src/to_html.rs +++ b/src/to_html.rs @@ -1457,7 +1457,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 + || (context.options.allow_any_img_src && media.image); + + let url = if allow_dangerous_protocol { sanitize(destination) } else { sanitize_with_protocols( diff --git a/tests/image.rs b/tests/image.rs index 19144d29..2827daff 100644 --- a/tests/image.rs +++ b/tests/image.rs @@ -235,6 +235,22 @@ fn image() -> Result<(), message::Message> { "should allow non-http protocols w/ `allowDangerousProtocol`" ); + 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 with the `allow_any_img_src` option" + ); + 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..733d936f 100644 --- a/tests/misc_dangerous_protocol.rs +++ b/tests/misc_dangerous_protocol.rs @@ -195,3 +195,34 @@ fn dangerous_protocol_link() { "should allow a colon in a path" ); } + +#[test] +fn dangerous_protocol_image_with_option() { + use markdown::{to_html_with_options, CompileOptions, Options}; + + let options = Options { + compile: CompileOptions { + allow_any_img_src: 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_any_img_src option" + ); + + let result = to_html_with_options("![](irc:///help)", &options).unwrap(); + assert_eq!( + result, "

\"\"

", + "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_any_img_src option" + ); +}