Skip to content

Commit

Permalink
Merge pull request #799 from dereklieu/xss-bug-fix
Browse files Browse the repository at this point in the history
XSS bug fix
  • Loading branch information
dereklieu committed Feb 5, 2015
2 parents c25dec7 + 78d1520 commit 9d2d4a0
Show file tree
Hide file tree
Showing 7 changed files with 33 additions and 26 deletions.
6 changes: 3 additions & 3 deletions app/views/file.js
Original file line number Diff line number Diff line change
Expand Up @@ -303,7 +303,7 @@ module.exports = Backbone.View.extend({
}
}).bind(this));

return content;
return _.escape(content);
},

initEditor: function() {
Expand Down Expand Up @@ -651,14 +651,14 @@ module.exports = Backbone.View.extend({
metadata = this.model.get('metadata') || {};
content = this.model.get('content') || '';
}

// Run the liquid parsing.
var parsedTemplate = Liquid.parse(this.compilePreview(content)).render({
site: this.collection.config,
post: metadata,
page: metadata
});

// If it's markdown, run it through marked; otherwise, leave it alone.
if(this.model.get('markdown')) parsedTemplate = marked(parsedTemplate);

Expand Down
6 changes: 3 additions & 3 deletions app/views/header.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,11 +53,11 @@ module.exports = Backbone.View.extend({
avatar: avatar,
repo: this.repo ? this.repo.attributes : undefined,
isPrivate: isPrivate,
input: this.input,
path: path,
input: _.escape(this.input),
path: _.escape(path),
placeholder: this.placeholder,
user: user,
title: title,
title: _.escape(title),
mode: this.title ? 'title' : 'path',
translate: this.file ? this.file.get('translate') : undefined
};
Expand Down
2 changes: 1 addition & 1 deletion app/views/sidebar/branch.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ module.exports = Backbone.View.extend({
this.$el.val('#' + [ this.repo.get('owner').login, this.repo.get('name'), 'tree', this.model.get('name') ].join('/'));
this.el.selected = this.branch && this.branch === this.model.get('name');

this.$el.html(this.model.get('name'));
this.$el.html(_.escape(this.model.get('name')));

return this;
}
Expand Down
14 changes: 7 additions & 7 deletions templates/li/file.html
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
<% if (file.binary) { %>
<div class='listing-icon icon round <%= file.extension %> <% if (file.media) { %>media<% } %>'></div>
<div class='listing-icon icon round <%- file.extension %> <% if (file.media) { %>media<% } %>'></div>
<% } else { %>
<a href='#<%= file.repo.owner.login %>/<%= file.repo.name %>/edit/<%= file.branch %>/<%= file.path %>' class='listing-icon'>
<span class='icon round <%= file.extension %> <% if (file.markdown) { %> md<% } %> <% if (file.media) { %> media<% } %>'></span>
<a href='#<%= file.repo.owner.login %>/<%= file.repo.name %>/edit/<%- file.branch %>/<%- file.path %>' class='listing-icon'>
<span class='icon round <%- file.extension %> <% if (file.markdown) { %> md<% } %> <% if (file.media) { %> media<% } %>'></span>
</a>
<% } %>

Expand All @@ -11,7 +11,7 @@
<% if (!file.binary) { %>
<a class='clearfix'
title="<%= t('main.repo.edit') %>"
href='#<%= file.repo.owner.login %>/<%= file.repo.name %>/edit/<%= file.branch %>/<%= file.path %>'>
href='#<%= file.repo.owner.login %>/<%= file.repo.name %>/edit/<%- file.branch %>/<%- file.path %>'>
<%= t('main.repo.edit') %>
</a>
<% } %>
Expand All @@ -25,9 +25,9 @@
<% } %>
</div>
<% if (file.binary) { %>
<h3 class='title' title='<%= file.name %>'><%= file.name %></h3>
<h3 class='title' title='<%- file.name %>'><%- file.name %></h3>
<% } else { %>
<h3 class='title' title='<%= file.name %>'><a class='clearfix'href='#<%= file.repo.owner.login %>/<%= file.repo.name %>/edit/<%= file.branch %>/<%= file.path %>'><%= file.name %></a></h3>
<h3 class='title' title='<%- file.name %>'><a class='clearfix'href='#<%= file.repo.owner.login %>/<%= file.repo.name %>/edit/<%- file.branch %>/<%- file.path %>'><%- file.name %></a></h3>
<% } %>
<span class='deemphasize'><%= file.jailpath %></span>
<span class='deemphasize'><%- file.jailpath %></span>
</div>
4 changes: 2 additions & 2 deletions templates/li/folder.html
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@
<span class='details'>
<h3 class='title' title='<%= folder.name %>'>
<a href='#<%= folder.repo.owner.login %>/<%= folder.repo.name %>/tree/<%= folder.branch %>/<%= folder.path %>'>
<%= folder.name %>
<%- folder.name %>
</a>
</h3>
<span class='deemphasize'><%= folder.jailpath %></span>
<span class='deemphasize'><%- folder.jailpath %></span>
</span>
4 changes: 2 additions & 2 deletions templates/sidebar/li/commit.html
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<a class='<%= data.status %>' href='#<%= [data.repo.owner.login, data.repo.name, data.mode, data.branch, data.path].join("/") %>'>
<a class='<%= data.status %>' href='#<%- [data.repo.owner.login, data.repo.name, data.mode, data.branch, data.path].join("/") %>'>
<span class='ico small inline <%= data.status %>'></span>
<span class='message'><%= data.file.filename %></span>
<span class='message'><%- data.file.filename %></span>
</a>
23 changes: 15 additions & 8 deletions test/spec/views/file.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,17 @@ var FileView = require('../../../app/views/file'),
mockFile = require('../../mocks/models/file'),
mockApp = require('../../mocks/views/app'),
mockRouter = require('../../mocks/router');


describe('File view', function() {
var fileView;

beforeEach(function() {
mockApp.reset(); // reset the DI state between tests.
});

describe('in edit mode', function() {

beforeEach(function() {
fileView = new FileView({
app: mockApp(),
Expand All @@ -30,15 +30,15 @@ describe('File view', function() {
sidebar: mockApp().sidebar
});
});

it('creates the CodeMirror editor', function() {
fileView.model = mockFile()
fileView.collection = fileView.model.collection;
fileView.render();

expect(fileView.editor).to.be.ok;
})

it('initializes CodeMirror with the file\'s contents', function() {
var content = 'the file contents';
fileView.model = mockFile(content)
Expand All @@ -47,5 +47,12 @@ describe('File view', function() {
expect(fileView.editor.getValue()).to.equal(content)
})
});


describe('in preview mode', function() {
it('escapes script tags when compiling preview', function() {
var content = "<script>alert('pwned')</script>";
expect(fileView.compilePreview(content)).to.equal('&lt;script&gt;alert(&#x27;pwned&#x27;)&lt;&#x2F;script&gt;');
});
});

});

0 comments on commit 9d2d4a0

Please sign in to comment.