Skip to content

Commit

Permalink
Version 1.6.1; fix
Browse files Browse the repository at this point in the history
 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 `<hi <script>attack<hi</script>`,
       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 `<br/>`'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.)
  • Loading branch information
David Parsons committed Feb 2, 2010
1 parent ace5dbb commit e0ce21f
Show file tree
Hide file tree
Showing 10 changed files with 76 additions and 20 deletions.
2 changes: 1 addition & 1 deletion VERSION
@@ -1 +1 @@
1.6.0
1.6.1
15 changes: 9 additions & 6 deletions generate.c
Expand Up @@ -898,7 +898,10 @@ maybe_tag_or_link(MMIOT *f)
else
size++;

Qstring(forbidden_tag(f) ? "&lt;" : "<", f);
if ( forbidden_tag(f) )
return 0;

Qchar('<', f);
while ( ((c = peek(f, 1)) != EOF) && (c != '>') )
Qchar(pull(f), f);
return 1;
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -1183,11 +1186,10 @@ text(MMIOT *f)
break;
case '<': Qstring("&lt;", 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:
Expand Down Expand Up @@ -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);
}
Expand Down
35 changes: 29 additions & 6 deletions markdown.c
Expand Up @@ -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;
}


Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -709,16 +730,18 @@ 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) {
q = t->next;
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) ) {
Expand Down
4 changes: 4 additions & 0 deletions tests/autolink.t
Expand Up @@ -27,6 +27,10 @@ try -fautolink 'single link' \
'http://www.pell.portland.or.us/~orc/Code/discount' \
'<p><a href="http://www.pell.portland.or.us/~orc/Code/discount">http://www.pell.portland.or.us/~orc/Code/discount</a></p>'

try -fautolink '[!](http://a.com "http://b.com")' \
'[!](http://a.com "http://b.com")' \
'<p><a href="http://a.com" title="http://b.com">!</a></p>'

try -fautolink 'link surrounded by text' \
'here http://it is?' \
'<p>here <a href="http://it">http://it</a> is?</p>'
Expand Down
4 changes: 4 additions & 0 deletions tests/header.t
Expand Up @@ -39,4 +39,8 @@ try 'single-char ETX (## W ##)' '## W ##' '<h2>W</h2>'

try 'multiple-char ETX (##Hello##)' '##Hello##' '<h2>Hello</h2>'

try 'SETEXT with trailing whitespace' \
'hello
===== ' '<h1>hello</h1>'

exit $rc
1 change: 1 addition & 0 deletions tests/html.t
Expand Up @@ -68,6 +68,7 @@ try 'no smartypants inside tags (#2)' \
'<p><img src="linky" alt=":)" /></p>'

try -fnohtml 'block html with -fnohtml' '<b>hi!</b>' '<p>&lt;b>hi!&lt;/b></p>'
try -fnohtml 'malformed tag injection' '<x <script>' '<p>&lt;x &lt;script></p>'
try -fhtml 'allow html with -fhtml' '<b>hi!</b>' '<p><b>hi!</b></p>'


Expand Down
12 changes: 11 additions & 1 deletion tests/list.t
Expand Up @@ -87,7 +87,7 @@ try 'nested lists (2)' \
</ol>
<p> Here</p></li>
<p> Here</p></li>
<li>B (list)</li>
</ul>'

Expand Down Expand Up @@ -134,6 +134,16 @@ try 'empty list' \
</ul>'


try 'blockquote inside a list' \
' * This is a list item.
> This is a quote insde a list item. ' \
'<ul>
<li><p> This is a list item.</p>
<blockquote><p>This is a quote insde a list item.</p></blockquote></li>
</ul>'

if ./markdown -V | grep DL_TAG >/dev/null; then

try 'dl followed by non-dl' \
Expand Down
10 changes: 6 additions & 4 deletions tests/peculiarities.t
Expand Up @@ -54,11 +54,13 @@ try 'ol with mixed item prefixes' \
<li>B</li>
</ol>'

try 'forcing a <br/>' 'this ' '<p>this<br/>
</p>'
try 'forcing a <br/>' 'this
is' '<p>this<br/>
is</p>'

try 'trimming single spaces' 'this ' '<p>this</p>'
try -fnohtml 'markdown <br/> with -fnohtml' 'foo ' '<p>foo<br/>
</p>'
try -fnohtml 'markdown <br/> with -fnohtml' 'foo
is' '<p>foo<br/>
is</p>'

exit $rc
11 changes: 9 additions & 2 deletions tests/schiraldi.t
Expand Up @@ -26,8 +26,9 @@ try() {
fi
}

try -fnohtml 'breaks with -fnohtml' 'foo ' '<p>foo<br/>
</p>'
try -fnohtml 'breaks with -fnohtml' 'foo
bar' '<p>foo<br/>
bar</p>'

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

Q="'"
try -fautolink 'security hole with \" in []()' \
'[XSS](/ "\"=\"\"onmouseover='$Q'alert(String.fromCharCode(88,83,83))'$Q'")' \
'<p><a href="/" title="\&quot;=\&quot;\&quot;onmouseover='$Q'alert(String.fromCharCode(88,83,83))'$Q'">XSS</a></p>'
exit $rc
2 changes: 2 additions & 0 deletions tests/snakepit.t
Expand Up @@ -39,5 +39,7 @@ try '<unfinished <tags> (1)' \
try '<unfinished &<tags> (2)' \
'<foo [bar](foo) &<s>hi</s>' \
'<p><foo [bar](foo) &<s>hi</s></p>'

try 'paragraph <br/> oddity' 'EOF ' '<p>EOF</p>'

exit $rc

0 comments on commit e0ce21f

Please sign in to comment.