User talk:CSteipp (WMF)/Training/VulnTagging medium

Answers

SQL Injection

Although the code uses mysql_real_escape_string, because the $articleId in "vt_article_id = $articleId" is not quoted, then escaping ' and \ marks will not prevent an attacker from adding SQL into the variable. For example:

<vtag articleid="103) UNION SELECT user_name, user_password from user WHERE 1=(1"></vtag>

Will result in the SQL statement: SELECT vt_tid,vt_tag_text FROM `vulntags` WHERE (vt_article_id = 103) UNION SELECT user_name, user_password from user WHERE 1=(1);

This is why you should pass this is a key-value pair, such as,

$res = $dbr->select(
	'vulntags',
	array( 'vt_tid', 'vt_tag_text' ),
	array( 'vt_article_id' => $articleId ),
	__METHOD__
)

XSS in Link

$otherpages = Linker::link(
	SpecialPage::getTitleFor( 'ArticlesWithTag', $tag->vt_tag_text ),
	$tag->vt_tag_text
);

Although Linker::link() is used to generate the html containing the $tag->vt_tag_text, the 2nd parameter is not escaped. Since the link text is in the dom text context, htmlspecialchars is appropriate for escaping:

$otherpages = Linker::link(
	SpecialPage::getTitleFor( 'ArticlesWithTag', $tag->vt_tag_text ),
	htmlspecialchars( $tag->vt_tag_text )
);

XSS in class attribute

$articleId = htmlspecialchars( $articleId );
return "<ul id='vuln-tag-list' class='tags-for-$articleId'>" . implode( "\n", $tags ) . "</ul>";

Although htmlspecialchars is used to escape the $articleId, it does not escape single quotes (') by default, so it is still possible to add a javascript handler to the ul element. This code should use the Html::rawElement() function instead, and allow MediaWiki to handle the escaping:

return Html::rawElement(
	'ul',
	array(
		'id' => 'vuln-tag-list',
		'class' => "tags-for-$articleId"
	),
	implode( "\n", $tags )
);

Add topic