User talk:CSteipp (WMF)/Training/VulnTagging medium
Add topicAppearance
Answers
[edit]- 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 ) );