Skip to content

Commit

Permalink
🔒 Fixed path traversal issue in theme files
Browse files Browse the repository at this point in the history
refs TryGhost/Product#2843

- Using encoded path traversal characters in URL's path allowed to fetch
  any file within active theme's folder, which is disallowed
- credits to: fuomag9 https://kiwi.fuo.fi/@fuomag9
  • Loading branch information
daniellockyer committed Apr 7, 2023
1 parent c99016f commit 378dd91
Show file tree
Hide file tree
Showing 2 changed files with 57 additions and 3 deletions.
36 changes: 33 additions & 3 deletions ghost/core/core/frontend/web/middleware/static-theme.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,45 @@ function isDeniedFile(file) {
return deniedFiles.includes(base) || deniedFileTypes.includes(ext);
}

/**
* Copy from:
* https://github.com/pillarjs/send/blob/b69cbb3dc4c09c37917d08a4c13fcd1bac97ade5/index.js#L987-L1003
*
* Allows V8 to only deoptimize this fn instead of all
* of send().
*
* @param {string} filePath
* @returns {string|number} returns -1 number if decode decodeURIComponent throws
*/
function decode(filePath) {
try {
return decodeURIComponent(filePath);
} catch (err) {
return -1;
}
}

/**
*
* @param {string} file path to a requested file
* @returns {boolean}
*/
function isAllowedFile(file) {
const decodedFilePath = decode(file);
if (decodedFilePath === -1) {
return false;
}

const normalizedFilePath = path.normalize(decodedFilePath);

const allowedFiles = ['manifest.json'];
const allowedPath = '/assets/';
const alwaysDeny = ['.hbs'];

const ext = path.extname(file);
const base = path.basename(file);
const ext = path.extname(normalizedFilePath);
const base = path.basename(normalizedFilePath);

return allowedFiles.includes(base) || (file.startsWith(allowedPath) && !alwaysDeny.includes(ext));
return allowedFiles.includes(base) || (normalizedFilePath.startsWith(allowedPath) && !alwaysDeny.includes(ext));
}

function forwardToExpressStatic(req, res, next) {
Expand Down
24 changes: 24 additions & 0 deletions ghost/core/test/unit/frontend/web/middleware/static-theme.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -184,4 +184,28 @@ describe('staticTheme', function () {
done();
});
});

it('should disallow path traversal', function (done) {
req.path = '/assets/built%2F..%2F..%2F/package.json';
req.method = 'GET';

staticTheme()(req, res, function next() {
activeThemeStub.called.should.be.false();
expressStaticStub.called.should.be.false();

done();
});
});

it('should not crash when malformatted URL sequence is passed', function (done) {
req.path = '/assets/built%2F..%2F..%2F%E0%A4%A/package.json';
req.method = 'GET';

staticTheme()(req, res, function next() {
activeThemeStub.called.should.be.false();
expressStaticStub.called.should.be.false();

done();
});
});
});

0 comments on commit 378dd91

Please sign in to comment.