Skip to content
Permalink
Browse files Browse the repository at this point in the history
Exclude more invalid chars from files UI path
Prevent newlines and zero byte chars to be used in files UI URL and
redirect to root if one is detected.

Added additional hardening in case the request fails with 400 or the
XMLHttpRequest throw a DOMException, both can happen with invalid paths
as well.
  • Loading branch information
Vincent Petry committed Oct 25, 2016
1 parent 942ca72 commit e7acbce
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 8 deletions.
32 changes: 25 additions & 7 deletions apps/files/js/filelist.js
Expand Up @@ -1329,11 +1329,18 @@

_isValidPath: function(path) {
var sections = path.split('/');
for (var i = 0; i < sections.length; i++) {
var i;
for (i = 0; i < sections.length; i++) {
if (sections[i] === '..') {
return false;
}
}
var specialChars = [decodeURIComponent('%00'), decodeURIComponent('%0A')];
for (i = 0; i < specialChars.length; i++) {
if (path.indexOf(specialChars[i]) !== -1) {
return false;
}
}
return true;
},

Expand All @@ -1345,6 +1352,7 @@
_setCurrentDir: function(targetDir, changeUrl) {
targetDir = targetDir.replace(/\\/g, '/');
if (!this._isValidPath(targetDir)) {
OC.Notification.showTemporary(t('files', 'Invalid path'));
targetDir = '/';
changeUrl = true;
}
Expand Down Expand Up @@ -1435,12 +1443,22 @@
this._currentFileModel = null;
this.$el.find('.select-all').prop('checked', false);
this.showMask();
this._reloadCall = this.filesClient.getFolderContents(
this.getCurrentDirectory(), {
includeParent: true,
properties: this._getWebdavProperties()
try {
this._reloadCall = this.filesClient.getFolderContents(
this.getCurrentDirectory(), {
includeParent: true,
properties: this._getWebdavProperties()
}
);
} catch (e) {
if (e instanceof DOMException) {
console.error(e);
this.changeDirectory('/');
OC.Notification.showTemporary(t('files', 'Invalid path'));
return;
}
);
throw e;
}
if (this._detailsView) {
// close sidebar
this._updateDetailsView(null);
Expand All @@ -1457,7 +1475,7 @@
}

// Firewall Blocked request?
if (status === 403) {
if (status === 403 || status === 400) {
// Go home
this.changeDirectory('/');
OC.Notification.showTemporary(t('files', 'This operation is forbidden'));
Expand Down
10 changes: 9 additions & 1 deletion apps/files/tests/js/filelistSpec.js
Expand Up @@ -1332,7 +1332,9 @@ describe('OCA.Files.FileList tests', function() {
'/../abc',
'/abc/..',
'/abc/../',
'/../abc/'
'/../abc/',
'/zero' + decodeURIComponent('%00') + 'byte/',
'/really who adds new' + decodeURIComponent('%0A') + 'lines in their paths/',
], function(path) {
fileList.changeDirectory(path);
expect(fileList.getCurrentDirectory()).toEqual('/');
Expand All @@ -1348,6 +1350,12 @@ describe('OCA.Files.FileList tests', function() {
expect(fileList.getCurrentDirectory()).toEqual(path);
});
});
it('switches to root dir in case of bad request', function() {
fileList.changeDirectory('/unexist');
// can happen in case of invalid chars in the URL
deferredList.reject(400);
expect(fileList.getCurrentDirectory()).toEqual('/');
});
it('switches to root dir when current directory does not exist', function() {
fileList.changeDirectory('/unexist');
deferredList.reject(404);
Expand Down

0 comments on commit e7acbce

Please sign in to comment.