From e0ce21fa566f1dcab9f0f8a73e092214d52f87ff Mon Sep 17 00:00:00 2001 From: David Parsons Date: Mon, 1 Feb 2010 22:28:32 -0800 Subject: [PATCH] Version 1.6.1; fix 1. two security holes (scripting attacks via stupid oversights that I never had test cases for) consisting of * if explicit html is forbidden, I was putting out a `<` then streaming out text until I reached a '>'. So if someone did `attack`, it would work, and * I was being overly greedy about escaping (with \\) `"` and `'`, so a malformed title string could contain arbitrary text, up to and including scripts. 2. three markdown.pl compatability cases * lines at the end of a paragraph never get `
`'ed, * blockquotes can be indented up to 3 spaces, * SETEXT-style header lines can have trailing spaces, and tweak one more markdown.pl compatability case, in that list items absorb new paragraphs if they're indented at all. The reference implementation is somewhat pathological about how it treats nested lists (in a way that I do not wish to follow) but I changed my "subsequent paragraph snarf" code in `markdown.c` to absorb paragraphs indented by at least the indent level of the list (so if the list is indented two spaces, following paragraphs indented two spaces will be absorbed.) --- VERSION | 2 +- generate.c | 15 +++++++++------ markdown.c | 35 +++++++++++++++++++++++++++++------ tests/autolink.t | 4 ++++ tests/header.t | 4 ++++ tests/html.t | 1 + tests/list.t | 12 +++++++++++- tests/peculiarities.t | 10 ++++++---- tests/schiraldi.t | 11 +++++++++-- tests/snakepit.t | 2 ++ 10 files changed, 76 insertions(+), 20 deletions(-) diff --git a/VERSION b/VERSION index dc1e644a..9c6d6293 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.6.0 +1.6.1 diff --git a/generate.c b/generate.c index 1133f4b3..a1355524 100644 --- a/generate.c +++ b/generate.c @@ -898,7 +898,10 @@ maybe_tag_or_link(MMIOT *f) else size++; - Qstring(forbidden_tag(f) ? "<" : "<", f); + if ( forbidden_tag(f) ) + return 0; + + Qchar('<', f); while ( ((c = peek(f, 1)) != EOF) && (c != '>') ) Qchar(pull(f), f); return 1; @@ -1079,7 +1082,7 @@ text(MMIOT *f) int smartyflags = 0; while (1) { - if ( (f->flags & AUTOLINK) && isalpha(peek(f,1)) ) + if ( (f->flags & AUTOLINK) && isalpha(peek(f,1)) && !tag_text(f) ) maybe_autolink(f); c = pull(f); @@ -1183,11 +1186,10 @@ text(MMIOT *f) break; case '<': Qstring("<", f); break; - case '\\': case '>': case '#': case '.': case '-': case '+': case '{': case '}': case ']': - case '(': case ')': case '"': case '\'': case '!': case '[': case '*': case '_': + case '\\':case '(': case ')': case '`': Qchar(c, f); break; default: @@ -1416,8 +1418,9 @@ printblock(Paragraph *pp, MMIOT *f) while (t) { if ( S(t->text) ) { - if ( S(t->text) > 2 && T(t->text)[S(t->text)-2] == ' ' - && T(t->text)[S(t->text)-1] == ' ') { + if ( t->next && S(t->text) > 2 + && T(t->text)[S(t->text)-2] == ' ' + && T(t->text)[S(t->text)-1] == ' ' ) { push(T(t->text), S(t->text)-2, f); push("\003\n", 2, f); } diff --git a/markdown.c b/markdown.c index 292512f7..90636018 100644 --- a/markdown.c +++ b/markdown.c @@ -315,7 +315,14 @@ isfootnote(Line *t) static int isquote(Line *t) { - return ( T(t->text)[0] == '>' ); + int j; + + for ( j=0; j < 4; j++ ) + if ( T(t->text)[j] == '>' ) + return 1; + else if ( !isspace(T(t->text)[j]) ) + return 0; + return 0; } @@ -382,9 +389,14 @@ ishdr(Line *t, int *htyp) if ( t->next ) { char *q = T(t->next->text); + int last = S(t->next->text); if ( (*q == '=') || (*q == '-') ) { - for (i=1; i < S(t->next->text); i++) + /* ignore trailing whitespace */ + while ( (last > 1) && isspace(q[last-1]) ) + --last; + + for (i=1; i < last; i++) if ( q[0] != q[i] ) return 0; *htyp = SETEXT; @@ -629,7 +641,16 @@ quoteblock(Paragraph *p) for ( t = p->text; t ; t = q ) { if ( isquote(t) ) { - qp = (T(t->text)[1] == ' ') ? 2 : 1; + /* clip leading spaces */ + for (qp = 0; T(t->text)[qp] != '>'; qp ++) + /* assert: the first nonblank character on this line + * will be a > + */; + /* clip '>' */ + qp++; + /* clip next space, if any */ + if ( T(t->text)[qp] == ' ' ) + qp++; CLIP(t->text, 0, qp); t->dle = mkd_firstnonblank(t); } @@ -709,8 +730,9 @@ listitem(Paragraph *p, int indent) } /* after a blank line, the next block needs to start with a line - * that's indented 4 spaces, but after that the line doesn't - * need any indentation + * that's indented 4(? -- reference implementation allows a 1 + * character indent, but that has unfortunate side effects here) + * spaces, but after that the line doesn't need any indentation */ if ( q != t->next ) { if (q->dle < indent) { @@ -718,7 +740,8 @@ listitem(Paragraph *p, int indent) t->next = 0; return q; } - indent = 4; + /* indent as far as the initial line was indented. */ + indent = clip; } if ( (q->dle < indent) && (ishr(q) || islist(q,&z)) && !ishdr(q,&z) ) { diff --git a/tests/autolink.t b/tests/autolink.t index f1cb56f4..a471fea8 100644 --- a/tests/autolink.t +++ b/tests/autolink.t @@ -27,6 +27,10 @@ try -fautolink 'single link' \ 'http://www.pell.portland.or.us/~orc/Code/discount' \ '

http://www.pell.portland.or.us/~orc/Code/discount

' +try -fautolink '[!](http://a.com "http://b.com")' \ + '[!](http://a.com "http://b.com")' \ + '

!

' + try -fautolink 'link surrounded by text' \ 'here http://it is?' \ '

here http://it is?

' diff --git a/tests/header.t b/tests/header.t index 6c24a1a8..aaab824d 100644 --- a/tests/header.t +++ b/tests/header.t @@ -39,4 +39,8 @@ try 'single-char ETX (## W ##)' '## W ##' '

W

' try 'multiple-char ETX (##Hello##)' '##Hello##' '

Hello

' +try 'SETEXT with trailing whitespace' \ +'hello +===== ' '

hello

' + exit $rc diff --git a/tests/html.t b/tests/html.t index 03d365d8..9f75895c 100644 --- a/tests/html.t +++ b/tests/html.t @@ -68,6 +68,7 @@ try 'no smartypants inside tags (#2)' \ '

:)

' try -fnohtml 'block html with -fnohtml' 'hi!' '

<b>hi!</b>

' +try -fnohtml 'malformed tag injection' '' '

<x <script>

' try -fhtml 'allow html with -fhtml' 'hi!' '

hi!

' diff --git a/tests/list.t b/tests/list.t index 77290b48..8ffacda2 100644 --- a/tests/list.t +++ b/tests/list.t @@ -87,7 +87,7 @@ try 'nested lists (2)' \ -

Here

+

Here

  • B (list)
    • ' @@ -134,6 +134,16 @@ try 'empty list' \ ' +try 'blockquote inside a list' \ +' * This is a list item. + + > This is a quote insde a list item. ' \ +'
      +

    • This is a list item.

      + +

      This is a quote insde a list item.

      • +
    ' + if ./markdown -V | grep DL_TAG >/dev/null; then try 'dl followed by non-dl' \ diff --git a/tests/peculiarities.t b/tests/peculiarities.t index 020c86b1..8a199bd3 100644 --- a/tests/peculiarities.t +++ b/tests/peculiarities.t @@ -54,11 +54,13 @@ try 'ol with mixed item prefixes' \
  • B
    • ' -try 'forcing a
    ' 'this ' '

    this
    -

    ' +try 'forcing a
    ' 'this +is' '

    this
    +is

    ' try 'trimming single spaces' 'this ' '

    this

    ' -try -fnohtml 'markdown
    with -fnohtml' 'foo ' '

    foo
    -

    ' +try -fnohtml 'markdown
    with -fnohtml' 'foo +is' '

    foo
    +is

    ' exit $rc diff --git a/tests/schiraldi.t b/tests/schiraldi.t index 5f519a9a..2d90813d 100644 --- a/tests/schiraldi.t +++ b/tests/schiraldi.t @@ -26,8 +26,9 @@ try() { fi } -try -fnohtml 'breaks with -fnohtml' 'foo ' '

    foo
    -

    ' +try -fnohtml 'breaks with -fnohtml' 'foo +bar' '

    foo
    +bar

    ' try 'links with trailing \)' \ '[foo](http://en.wikipedia.org/wiki/Link_(film\))' \ @@ -94,4 +95,10 @@ try -fautolink 'autolink url with : & ;' \ 'http://www.biblegateway.com/passage/?search=Matthew%205:29-30;&version=31;' \ '

    http://www.biblegateway.com/passage/?search=Matthew%205:29-30;&version=31;

    ' +Q="'" +try -fautolink 'security hole with \" in []()' \ +'[XSS](/ "\"=\"\"onmouseover='$Q'alert(String.fromCharCode(88,83,83))'$Q'")' \ +'

    XSS

    ' + + exit $rc diff --git a/tests/snakepit.t b/tests/snakepit.t index da04e528..886806fe 100644 --- a/tests/snakepit.t +++ b/tests/snakepit.t @@ -39,5 +39,7 @@ try ' (1)' \ try ' (2)' \ 'hi' \ '

    hi

    ' + +try 'paragraph
    oddity' 'EOF ' '

    EOF

    ' exit $rc