Skip to content

Commit e923a3c

Browse files
authored
Add allow_any_img_src option
Closes GH-164. Closes GH-165. Reviewed-by: Titus Wormer <[email protected]>
1 parent 283db1a commit e923a3c

File tree

5 files changed

+122
-2
lines changed

5 files changed

+122
-2
lines changed

readme.md

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -285,12 +285,21 @@ The following bash scripts are useful when working on this project:
285285
The typical security aspect discussed for markdown is [cross-site scripting
286286
(XSS)][xss] attacks.
287287
Markdown itself is safe if it does not include embedded HTML or dangerous
288-
protocols in links/images (such as `javascript:` or `data:`).
288+
protocols in links/images (such as `javascript:`).
289289
`markdown-rs` makes any markdown safe by default, even if HTML is embedded or
290290
dangerous protocols are used, as it encodes or drops them.
291+
291292
Turning on the `allow_dangerous_html` or `allow_dangerous_protocol` options for
292293
user-provided markdown opens you up to XSS attacks.
293294

295+
Additionnally, you should be able to set `allow_any_img_src` safely.
296+
The default is to allow only `http:`, `https:`, and relative images,
297+
which is what GitHub does. But it should be safe to allow any value on `src`.
298+
299+
The [HTML specification][whatwg-html-image] prohibits dangerous scripts in
300+
images and all modern browsers respect this and are thus safe.
301+
Opera 12 (from 2012) is a notable browser that did not respect this.
302+
294303
An aspect related to XSS for security is syntax errors: markdown itself has no
295304
syntax errors.
296305
Some syntax extensions (specifically, only MDX) do include syntax errors.
@@ -413,3 +422,5 @@ Special thanks go out to:
413422
[support]: .github/support.md
414423

415424
[coc]: .github/code-of-conduct.md
425+
426+
[whatwg-html-image]: https://html.spec.whatwg.org/multipage/images.html#images-processing-model

src/configuration.rs

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -522,6 +522,16 @@ pub struct CompileOptions {
522522
/// `ircs`, `mailto`, `xmpp`), are safe.
523523
/// All other URLs are dangerous and dropped.
524524
///
525+
/// When the option `allow_all_protocols_in_img` is enabled,
526+
/// `allow_dangerous_protocol` only applies to links.
527+
///
528+
/// This is safe because the
529+
/// [HTML specification][whatwg-html-image-processing]
530+
/// does not allow executable code in images.
531+
/// All modern browsers respect this.
532+
///
533+
/// [whatwg-html-image-processing]: https://html.spec.whatwg.org/multipage/images.html#images-processing-model
534+
///
525535
/// ## Examples
526536
///
527537
/// ```
@@ -553,6 +563,55 @@ pub struct CompileOptions {
553563
/// ```
554564
pub allow_dangerous_protocol: bool,
555565

566+
/// Whether to allow all values in images.
567+
///
568+
/// The default is `false`,
569+
/// which lets `allow_dangerous_protocol` control protocol safety for
570+
/// both links and images.
571+
///
572+
/// Pass `true` to allow all values as `src` on images,
573+
/// regardless of `allow_dangerous_protocol`.
574+
/// This is safe because the
575+
/// [HTML specification][whatwg-html-image-processing]
576+
/// does not allow executable code in images.
577+
///
578+
/// [whatwg-html-image-processing]: https://html.spec.whatwg.org/multipage/images.html#images-processing-model
579+
///
580+
/// ## Examples
581+
///
582+
/// ```
583+
/// use markdown::{to_html_with_options, CompileOptions, Options};
584+
/// # fn main() -> Result<(), markdown::message::Message> {
585+
///
586+
/// // By default, some protocols in image sources are dropped:
587+
/// assert_eq!(
588+
/// to_html_with_options(
589+
/// "![](data:image/gif;base64,R0lGODlhAQABAAAAACH5BAEKAAEALAAAAAABAAEAAAICTAEAOw==)",
590+
/// &Options::default()
591+
/// )?,
592+
/// "<p><img src=\"\" alt=\"\" /></p>"
593+
/// );
594+
///
595+
/// // Turn `allow_any_img_src` on to allow all values as `src` on images.
596+
/// // This is safe because browsers do not execute code in images.
597+
/// assert_eq!(
598+
/// to_html_with_options(
599+
/// "![](javascript:alert(1))",
600+
/// &Options {
601+
/// compile: CompileOptions {
602+
/// allow_any_img_src: true,
603+
/// ..CompileOptions::default()
604+
/// },
605+
/// ..Options::default()
606+
/// }
607+
/// )?,
608+
/// "<p><img src=\"javascript:alert(1)\" alt=\"\" /></p>"
609+
/// );
610+
/// # Ok(())
611+
/// # }
612+
/// ```
613+
pub allow_any_img_src: bool,
614+
556615
// To do: `doc_markdown` is broken.
557616
#[allow(clippy::doc_markdown)]
558617
/// Default line ending to use when compiling to HTML, for line endings not

src/to_html.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1457,7 +1457,10 @@ fn on_exit_media(context: &mut CompileContext) {
14571457
};
14581458

14591459
if let Some(destination) = destination {
1460-
let url = if context.options.allow_dangerous_protocol {
1460+
let allow_dangerous_protocol = context.options.allow_dangerous_protocol
1461+
|| (context.options.allow_any_img_src && media.image);
1462+
1463+
let url = if allow_dangerous_protocol {
14611464
sanitize(destination)
14621465
} else {
14631466
sanitize_with_protocols(

tests/image.rs

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -235,6 +235,22 @@ fn image() -> Result<(), message::Message> {
235235
"should allow non-http protocols w/ `allowDangerousProtocol`"
236236
);
237237

238+
assert_eq!(
239+
to_html_with_options(
240+
"![](javascript:alert(1))",
241+
&Options {
242+
compile: CompileOptions {
243+
allow_dangerous_protocol: false,
244+
allow_any_img_src: true,
245+
..Default::default()
246+
},
247+
..Default::default()
248+
}
249+
)?,
250+
"<p><img src=\"javascript:alert(1)\" alt=\"\" /></p>",
251+
"should allow non-http protocols with the `allow_any_img_src` option"
252+
);
253+
238254
assert_eq!(
239255
to_mdast(
240256
"a ![alpha]() b ![bravo](charlie 'delta') c.",

tests/misc_dangerous_protocol.rs

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -195,3 +195,34 @@ fn dangerous_protocol_link() {
195195
"should allow a colon in a path"
196196
);
197197
}
198+
199+
#[test]
200+
fn dangerous_protocol_image_with_option() {
201+
use markdown::{to_html_with_options, CompileOptions, Options};
202+
203+
let options = Options {
204+
compile: CompileOptions {
205+
allow_any_img_src: true,
206+
..Default::default()
207+
},
208+
..Default::default()
209+
};
210+
211+
let result = to_html_with_options("![](javascript:alert(1))", &options).unwrap();
212+
assert_eq!(
213+
result, "<p><img src=\"javascript:alert(1)\" alt=\"\" /></p>",
214+
"should allow javascript protocol with allow_any_img_src option"
215+
);
216+
217+
let result = to_html_with_options("![](irc:///help)", &options).unwrap();
218+
assert_eq!(
219+
result, "<p><img src=\"irc:///help\" alt=\"\" /></p>",
220+
"should allow irc protocol with allow_any_img_src option"
221+
);
222+
223+
let result = to_html_with_options("![](mailto:a)", &options).unwrap();
224+
assert_eq!(
225+
result, "<p><img src=\"mailto:a\" alt=\"\" /></p>",
226+
"should allow mailto protocol with allow_any_img_src option"
227+
);
228+
}

0 commit comments

Comments
 (0)