Navigation Menu

Skip to content

Commit

Permalink
Fix a comment vulnerability allowing scripts to be loaded.
Browse files Browse the repository at this point in the history
Due to the way that comments were loaded in, it was possible to
terminate a script and inject a new one while loading the diff viewer.
This isn't believed to have been a problem in the wild, but is certainly
an important one to fix.

We now ensure that the text is escaped at the point where it's being fed
into the JavaScript. It's no longer possible to inject scripts.

Thanks to Damian Johnson for the heads up and for the fix that this
change is based on.

This will be going into 1.5.7 and 1.6.3 releases.
  • Loading branch information
chipx86 committed Nov 15, 2011
1 parent 9b789e4 commit 7a0a9d9
Show file tree
Hide file tree
Showing 3 changed files with 9 additions and 2 deletions.
3 changes: 3 additions & 0 deletions reviewboard/htdocs/media/rb/js/diffviewer.js
Expand Up @@ -160,6 +160,9 @@ function DiffCommentBlock(beginRow, endRow, beginLineNum, endLineNum,
for (var i in comments) {
var comment = comments[i];

// We load in encoded text, so decode it.
comment.text = $("<div/>").html(comment.text).text();

if (comment.localdraft) {
this._createDraftComment(comment.text);
} else {
Expand Down
3 changes: 3 additions & 0 deletions reviewboard/htdocs/media/rb/js/screenshots.js
Expand Up @@ -36,6 +36,9 @@ function CommentBlock(x, y, width, height, container, comments) {
for (var i in comments) {
var comment = comments[i];

// We load in encoded text, so decode it.
comment.text = $("<div/>").html(comment.text).text();

if (comment.localdraft) {
this._createDraftComment(comment.text);
} else {
Expand Down
5 changes: 3 additions & 2 deletions reviewboard/reviews/templatetags/reviewtags.py
Expand Up @@ -4,6 +4,7 @@
from django.template import NodeList, TemplateSyntaxError
from django.template.loader import render_to_string
from django.utils import simplejson
from django.utils.html import escape
from django.utils.translation import ugettext_lazy as _
from djblets.util.decorators import basictag, blocktag
from djblets.util.misc import get_object_or_none
Expand Down Expand Up @@ -126,7 +127,7 @@ def commentcounts(context, filediff, interfilediff=None):

comment_dict.setdefault(key, []).append({
'comment_id': comment.id,
'text': comment.text,
'text': escape(comment.text),
'line': comment.first_line,
'num_lines': comment.num_lines,
'user': {
Expand Down Expand Up @@ -185,7 +186,7 @@ def screenshotcommentcounts(context, screenshot):

comments.setdefault(position, []).append({
'id': comment.id,
'text': comment.text,
'text': escape(comment.text),
'user': {
'username': review.user.username,
'name': review.user.get_full_name() or review.user.username,
Expand Down

0 comments on commit 7a0a9d9

Please sign in to comment.