diff --git a/app/background-process/networks/dat/web-api.js b/app/background-process/networks/dat/web-api.js index 4e4a7ec1d3..fce7b07dfb 100644 --- a/app/background-process/networks/dat/web-api.js +++ b/app/background-process/networks/dat/web-api.js @@ -9,6 +9,7 @@ import { queryPermission, requestPermission } from '../../ui/permissions' import { DAT_HASH_REGEX, DAT_QUOTA_DEFAULT_BYTES_ALLOWED, + DAT_VALID_PATH_REGEX, UserDeniedError, PermissionsError, @@ -26,7 +27,8 @@ import { ProtectedFileNotWritableError, FileAlreadyExistsError, FolderAlreadyExistsError, - ParentFolderDoesntExistError + ParentFolderDoesntExistError, + InvalidPathError } from '../../../lib/const' const DEFAULT_TIMEOUT = 5e3 @@ -184,6 +186,7 @@ export default { var senderOrigin = archivesDb.extractOrigin(this.sender.getURL()) yield assertWritePermission(archive, this.sender) yield assertQuotaPermission(archive, senderOrigin, Buffer.byteLength(data, opts.encoding)) + yield assertValidFilePath(filepath) return new Promise((resolve, reject) => { // protected files if (isProtectedFilePath(filepath)) { @@ -231,6 +234,7 @@ export default { createDirectory: m(function * (url) { var { archive, filepath } = lookupArchive(url) yield assertWritePermission(archive, this.sender) + yield assertValidPath(filepath) return new Promise((resolve, reject) => { // protected files if (isProtectedFilePath(filepath)) { @@ -316,7 +320,7 @@ function checkProtocolPermission (sender) { // helper to check if filepath refers to a file that userland is not allowed to edit directly function isProtectedFilePath (filepath) { - return filepath === '/' || filepath === '/dat.json' + return filepath === '/dat.json' } function assertWritePermission (archive, sender) { @@ -356,6 +360,19 @@ function assertQuotaPermission (archive, senderOrigin, byteLength) { }) } +function assertValidFilePath (filepath) { + return co(function * () { + if (filepath.slice(-1) === '/') throw new InvalidPathError('Files can not have a trailing slash') + yield assertValidPath (filepath) + }) +} + +function assertValidPath (fileOrFolderPath) { + return co(function * () { + if (!DAT_VALID_PATH_REGEX.test(fileOrFolderPath)) throw new InvalidPathError('Path contains invalid characters') + }) +} + // helper to handle the URL argument that's given to most args // - can get a dat hash, or dat url // - returns { archive, filepath } diff --git a/app/lib/const.js b/app/lib/const.js index 72550cea61..6893711091 100644 --- a/app/lib/const.js +++ b/app/lib/const.js @@ -6,6 +6,9 @@ export const BKR_SERVER_PORT = 17760 // 64 char hex export const DAT_HASH_REGEX = /^[0-9a-f]{64}$/i +// url file paths +export const DAT_VALID_PATH_REGEX = /^[a-z0-9\-._~!$&'()*+,;=:@\/]+$/i + // dat settings export const DAT_MANIFEST_FILENAME = 'dat.json' export const DAT_QUOTA_DEFAULT_BYTES_ALLOWED = (process.env.beaker_dat_quota_default_bytes_allowed || 104857600) // 100mb @@ -35,6 +38,7 @@ export const ProtectedFileNotWritableError = zerr('ProtectedFileNotWritableError export const FileAlreadyExistsError = zerr('FileAlreadyExistsError') export const FolderAlreadyExistsError = zerr('FolderAlreadyExistsError') export const ParentFolderDoesntExistError = zerr('ParentFolderDoesntExistError') +export const InvalidPathError = zerr('InvalidPathError') // dat network config export const DAT_DOMAIN = 'dat.local' diff --git a/app/shell-window/ui/navbar.js b/app/shell-window/ui/navbar.js index cf473f9641..dd05cba090 100644 --- a/app/shell-window/ui/navbar.js +++ b/app/shell-window/ui/navbar.js @@ -4,7 +4,7 @@ import * as pages from '../pages' import * as zoom from '../pages/zoom' import * as yo from 'yo-yo' import emitStream from 'emit-stream' -import * as prettyHash from 'pretty-hash' +import prettyHash from 'pretty-hash' import { UpdatesNavbarBtn } from './navbar/updates' import { DropMenuNavbarBtn } from './navbar/drop-menu' import { SiteInfoNavbarBtn } from './navbar/site-info' @@ -278,18 +278,20 @@ function render (id, page) { } function renderPrettyLocation (value, isHidden) { - var valueRendered + var valueRendered = value if (/^(dat|http|https):/.test(value)) { - var { protocol, host, pathname, search, hash } = new URL(value) - if (protocol === 'dat:' && isDatHashRegex.test(host)) host = prettyHash(host) - valueRendered = [ - yo`${protocol.slice(0, -1)}`, - yo`://`, - yo`${host}`, - yo`${pathname}${search}${hash}`, - ] - } else { - valueRendered = value + try { + var { protocol, host, pathname, search, hash } = new URL(value) + if (protocol === 'dat:' && isDatHashRegex.test(host)) host = prettyHash(host) + valueRendered = [ + yo`${protocol.slice(0, -1)}`, + yo`://`, + yo`${host}`, + yo`${pathname}${search}${hash}`, + ] + } catch (e) { + // invalid URL, just use value + } } return yo`
@@ -299,6 +301,7 @@ function renderPrettyLocation (value, isHidden) { function handleAutocompleteSearch (results) { var v = autocompleteCurrentValue + if (!v) return // decorate result with bolded regions // explicitly replace special characters to match sqlite fts tokenization diff --git a/tests/dat-web-api-test.js b/tests/dat-web-api-test.js index 269de81cdd..d812cf6294 100644 --- a/tests/dat-web-api-test.js +++ b/tests/dat-web-api-test.js @@ -281,13 +281,30 @@ test('dat.writeFile does not write to nonexistent directories', async t => { t.deepEqual(res.value.name, 'ParentFolderDoesntExistError') }) -test('dat.writeFile protects the root and manifest', async t => { - // write to the top-level folder +test('dat.writeFile gives an error for malformed names', async t => { + // write to the root dir var res = await app.client.executeAsync((url, done) => { dat.writeFile(url, 'hello world', 'utf8').then(done, done) - }, createdDatURL) - t.deepEqual(res.value.name, 'ProtectedFileNotWritableError') + }, createdDatURL + '/') + t.deepEqual(res.value.name, 'InvalidPathError') + t.deepEqual(res.value.message, 'Files can not have a trailing slash') + + // write to a subdir + var res = await app.client.executeAsync((url, done) => { + dat.writeFile(url, 'hello world', 'utf8').then(done, done) + }, createdDatURL + 'subdir/hello.txt/') + t.deepEqual(res.value.name, 'InvalidPathError') + t.deepEqual(res.value.message, 'Files can not have a trailing slash') + + // write with a bad char + var res = await app.client.executeAsync((url, done) => { + dat.writeFile(url, 'hello world', 'utf8').then(done, done) + }, createdDatURL + 'hello`.txt') + t.deepEqual(res.value.name, 'InvalidPathError') + t.deepEqual(res.value.message, 'Path contains invalid characters') +}) +test('dat.writeFile protects the manifest', async t => { // write to the manifest var res = await app.client.executeAsync((url, done) => { dat.writeFile(url, 'hello world', 'utf8').then(done, done) @@ -331,6 +348,12 @@ test('dat.writeFile doesnt overwrite folders', async t => { }) test('dat.createDirectory doesnt overwrite files or folders', async t => { + // write to the subdir + var res = await app.client.executeAsync((url, done) => { + dat.createDirectory(url).then(done, done) + }, createdDatURL + '/') + t.deepEqual(res.value.name, 'FolderAlreadyExistsError') + // write to the subdir var res = await app.client.executeAsync((url, done) => { dat.createDirectory(url).then(done, done) @@ -344,6 +367,15 @@ test('dat.createDirectory doesnt overwrite files or folders', async t => { t.deepEqual(res.value.name, 'FileAlreadyExistsError') }) +test('dat.createDirectory gives an error for malformed names', async t => { + // write with a bad char + var res = await app.client.executeAsync((url, done) => { + dat.createDirectory(url).then(done, done) + }, createdDatURL + 'hello`world') + t.deepEqual(res.value.name, 'InvalidPathError') + t.deepEqual(res.value.message, 'Path contains invalid characters') +}) + test('dat.writeFile doesnt allow writes that exceed the quota', async t => { // write to the subdir var res = await app.client.executeAsync((url, done) => { diff --git a/tests/lib/browser-driver.js b/tests/lib/browser-driver.js index 36f6468676..f3ba23aa01 100644 --- a/tests/lib/browser-driver.js +++ b/tests/lib/browser-driver.js @@ -1,7 +1,10 @@ exports.navigateTo = function (app, url) { return app.client.windowByIndex(0) .then(() => app.client.waitForExist('.toolbar-actions:not(.hidden) .nav-location-input')) - .then(() => app.client.click('.toolbar-actions:not(.hidden) .nav-location-input')) + .then(() => { + return app.client.click('.toolbar-actions:not(.hidden) .nav-location-pretty') + .catch(() => app.client.click('.toolbar-actions:not(.hidden) .nav-location-input')) + }) .then(() => app.client.setValue('.toolbar-actions:not(.hidden) .nav-location-input', url)) .then(() => app.client.pause(500)) // need to wait a sec for the UI to catch up .then(() => app.client.keys('\uE007')) // enter