fix: reject absolute file paths

apps/http-server/packages/generic/src/storage/fileStorage.ts

@@ -4,7 +4,7 @@ import { promisify } from 'util'
 import mime from 'mime-types'
 import prettyBytes from 'pretty-bytes'
 import { asyncPipe, CTXPost } from '../lib'
-import { HTTPServerConfig, LoggerInstance } from '@sofie-package-manager/api'
+import { betterPathResolve, HTTPServerConfig, LoggerInstance } from '@sofie-package-manager/api'
 import { BadResponse, PackageInfo, ResponseMeta, Storage } from './storage'
 import { Readable } from 'stream'
 
@@ -31,7 +31,7 @@ export class FileStorage extends Storage {
 	private logger: LoggerInstance
 	constructor(logger: LoggerInstance, private config: HTTPServerConfig) {
 		super()
-		this._basePath = path.resolve(this.config.httpServer.basePath)
+		this._basePath = betterPathResolve(this.config.httpServer.basePath)
 		this.logger = logger.category('FileStorage')
 
 		// Run this on startup, so that if there are any critical errors we'll see them right away:

shared/packages/api/src/__tests__/lib.spec.ts

@@ -1,4 +1,15 @@
-import { deferGets, diff, promiseTimeout, stringMaxLength, stringifyError, waitTime } from '../lib'
+import * as path from 'path'
+import {
+	betterPathIsAbsolute,
+	betterPathJoin,
+	betterPathResolve,
+	deferGets,
+	diff,
+	promiseTimeout,
+	stringMaxLength,
+	stringifyError,
+	waitTime,
+} from '../lib'
 
 describe('lib', () => {
 	test('diff', () => {
@@ -226,5 +237,38 @@ describe('lib', () => {
 		expect(stringMaxLength('0123456789abcdefg', 9)).toBe('012...efg')
 		expect(stringMaxLength('0123456789abcdefg', 8)).toBe('01...efg')
 	})
+	test('betterPathJoin', () => {
+		const fixSep = (str: string) => str.replace(/\//g, path.sep)
+
+		expect(betterPathJoin('\\\\a\\b/c')).toBe(fixSep('//a/b/c'))
+		expect(betterPathJoin('a/b', 'c')).toBe(fixSep('a/b/c'))
+		expect(betterPathJoin('a/b', '../c')).toBe(fixSep('a/c'))
+		expect(betterPathJoin('a/b', '/c')).toBe(fixSep('a/b/c'))
+		expect(betterPathJoin('C:\\a\\b', 'c\\d')).toBe(fixSep('C:/a/b/c/d'))
+		expect(betterPathJoin('C:\\a\\b', '../c')).toBe(fixSep('C:/a/c'))
+
+		expect(betterPathJoin('\\\\a\\b', '../c')).toBe(fixSep('//a/c')) // This is where path.join fails
+		expect(betterPathJoin('//a/b', '../c')).toBe(fixSep('//a/c')) // This is where path.join fails
+	})
+	test('betterPathResolve', () => {
+		const fixSep = (str: string) => str.replace(/\//g, path.sep)
+
+		expect(betterPathResolve('a/b/c')).toBe(path.resolve(fixSep('a/b/c')))
+		expect(betterPathResolve('a/b/c')).toBe(path.resolve(fixSep('a/b/c')))
+
+		expect(betterPathResolve('\\\\a\\b')).toBe(fixSep('//a/b'))
+		expect(betterPathResolve('\\\\a\\b\\c')).toBe(fixSep('//a/b/c'))
+	})
+	test('betterPathIsAbsolute', () => {
+		expect(betterPathIsAbsolute('a/b/c')).toBe(false)
+		expect(betterPathIsAbsolute('./a/b/c')).toBe(false)
+		expect(betterPathIsAbsolute('../a/b/c')).toBe(false)
+
+		expect(betterPathIsAbsolute('/a')).toBe(true)
+		expect(betterPathIsAbsolute('C:\\a\\b\\c')).toBe(true)
+		expect(betterPathIsAbsolute('\\\\a\\b\\c')).toBe(true)
+		expect(betterPathIsAbsolute('\\a')).toBe(true)
+		expect(betterPathIsAbsolute('//a')).toBe(true)
+	})
 })
 export {}

shared/packages/api/src/lib.ts

@@ -1,4 +1,5 @@
 import crypto from 'crypto'
+import path from 'path'
 import { compact } from 'underscore'
 import { AnyProtectedString } from './ProtectedString'
 
@@ -382,6 +383,54 @@ export function mapToObject<T>(map: Map<any, T>): { [key: string]: T } {
 	})
 	return o
 }
+/**
+ * Like path.join(),
+ * but this fixes an issue in path.join where it doesn't handle paths double slashes together with relative paths correctly
+ * And it also returns predictable results on both Windows and Linux
+ */
+export function betterPathJoin(...paths: string[]): string {
+	// Replace slashes with the correct path separator, because "C:\\a\\b" is not interpreted correctly on Linux (but C:/a/b is)
+	paths = paths.map((p) => p.replace(/[\\/]/g, path.sep))
+
+	let firstPath = paths[0]
+	const restPaths = paths.slice(1)
+
+	let prefix = ''
+	if (firstPath.startsWith('//') || firstPath.startsWith('\\\\')) {
+		// Preserve the prefix, as path.join will remove it:
+		prefix = path.sep + path.sep
+		firstPath = firstPath.slice(2)
+	}
+
+	return prefix + path.join(firstPath, ...restPaths)
+}
+/**
+ * Like path.resolve(),
+ * but it returns predictable results on both Windows and Linux
+ */
+export function betterPathResolve(p: string): string {
+	p = p.replace(/[\\/]/g, path.sep)
+
+	// let prefix = ''
+	if (p.startsWith('//') || p.startsWith('\\\\')) {
+		return path.sep + path.sep + path.normalize(p.slice(2))
+	} else {
+		return path.resolve(p)
+	}
+}
+/**
+ * Like path.isAbsolute(),
+ * but it returns same results on both Windows and Linux
+ */
+export function betterPathIsAbsolute(p: string): boolean {
+	return (
+		path.isAbsolute(p) ||
+		Boolean(p.match(/^\w:/)) || // C:\, path.isAbsolute() doesn't handle this on Linux
+		p.startsWith('\\\\') || // \\server\path, path.isAbsolute() doesn't handle this on Linux
+		p.startsWith('\\') // \server\path, path.isAbsolute() doesn't handle this on Linux
+	)
+}
+
 /** Returns true if we're running tests (in Jest) */
 export function isRunningInTest(): boolean {
 	// Note: JEST_WORKER_ID is set when running in unit tests

shared/packages/worker/src/worker/accessorHandlers/__tests__/fileShare.spec.ts

@@ -0,0 +1,82 @@
+import {
+	AccessorOnPackage,
+	protectString,
+	setupLogger,
+	initializeLogger,
+	ProcessConfig,
+	Expectation,
+	Accessor,
+} from '@sofie-package-manager/api'
+import { Content, FileShareAccessorHandle } from '../fileShare'
+import { PassiveTestWorker } from './lib'
+
+const processConfig: ProcessConfig = {
+	logPath: undefined,
+	logLevel: undefined,
+	unsafeSSL: false,
+	certificates: [],
+}
+initializeLogger({ process: processConfig })
+test('checkHandleBasic', () => {
+	const logger = setupLogger(
+		{
+			process: processConfig,
+		},
+		''
+	)
+	const worker = new PassiveTestWorker(logger)
+
+	function getFileShareAccessor(
+		accessor: AccessorOnPackage.FileShare,
+		content: Content,
+		workOptions: Expectation.WorkOptions.Base &
+			Expectation.WorkOptions.RemoveDelay &
+			Expectation.WorkOptions.UseTemporaryFilePath = {}
+	) {
+		accessor.type = Accessor.AccessType.FILE_SHARE
+		return new FileShareAccessorHandle(worker, protectString('share0'), accessor, content, workOptions)
+	}
+
+	expect(() => getFileShareAccessor({}, {}).checkHandleBasic()).toThrowError('Bad input data')
+
+	// missing accessor.folderPath:
+	expect(getFileShareAccessor({}, { filePath: 'amb.amp4' }).checkHandleBasic()).toMatchObject({
+		success: false,
+		reason: { tech: 'Folder path not set' },
+	})
+
+	// All OK:
+	expect(
+		getFileShareAccessor({ folderPath: '\\\\nas01\\media' }, { filePath: 'amb.amp4' }).checkHandleBasic()
+	).toMatchObject({
+		success: true,
+	})
+
+	// Absolute file path:
+	expect(
+		getFileShareAccessor({ folderPath: '\\\\nas01\\media' }, { filePath: '//secret/amb.amp4' }).checkHandleBasic()
+	).toMatchObject({
+		success: false,
+		reason: { tech: expect.stringMatching(/File path.*absolute path/) },
+	})
+	expect(
+		getFileShareAccessor(
+			{ folderPath: '\\\\nas01\\media' },
+			{ filePath: 'C:\\secret\\amb.amp4' }
+		).checkHandleBasic()
+	).toMatchObject({
+		success: false,
+		reason: { tech: expect.stringMatching(/File path.*absolute path/) },
+	})
+
+	// File path outside of folder path:
+	expect(
+		getFileShareAccessor({ folderPath: '//nas01/media' }, { filePath: '../secret/amb.amp4' }).checkHandleBasic()
+	).toMatchObject({
+		success: false,
+		reason: {
+			user: `File path is outside of folder path`,
+			tech: expect.stringMatching(/Full path.*does not start with/),
+		},
+	})
+})

shared/packages/worker/src/worker/accessorHandlers/__tests__/lib.ts

@@ -0,0 +1,65 @@
+import { protectString, LoggerInstance } from '@sofie-package-manager/api'
+import { BaseWorker } from '../../worker'
+
+export class PassiveTestWorker extends BaseWorker {
+	constructor(logger: LoggerInstance) {
+		super(
+			logger,
+			{
+				config: {
+					workerId: protectString('test'),
+					sourcePackageStabilityThreshold: 0,
+					windowsDriveLetters: ['X', 'Y', 'Z'],
+				},
+				location: {
+					// localComputerId?: string
+					localNetworkIds: [],
+				},
+				workerStorageWrite: () => {
+					throw new Error('Method not implemented.')
+				},
+				workerStorageRead: () => {
+					throw new Error('Method not implemented.')
+				},
+			},
+			async () => {
+				throw new Error('Method not implemented.')
+			},
+			'passive-test-worker'
+		)
+	}
+
+	async init() {
+		throw new Error('Method not implemented.')
+	}
+	terminate() {
+		throw new Error('Method not implemented.')
+	}
+	async doYouSupportExpectation(): Promise<any> {
+		throw new Error('Method not implemented.')
+	}
+	async getCostFortExpectation(): Promise<any> {
+		throw new Error('Method not implemented.')
+	}
+	async isExpectationReadyToStartWorkingOn(): Promise<any> {
+		throw new Error('Method not implemented.')
+	}
+	async isExpectationFulfilled(): Promise<any> {
+		throw new Error('Method not implemented.')
+	}
+	async workOnExpectation(): Promise<any> {
+		throw new Error('Method not implemented.')
+	}
+	async removeExpectation(): Promise<any> {
+		throw new Error('Method not implemented.')
+	}
+	async doYouSupportPackageContainer(): Promise<any> {
+		throw new Error('Method not implemented.')
+	}
+	async runPackageContainerCronJob(): Promise<any> {
+		throw new Error('Method not implemented.')
+	}
+	async setupPackageContainerMonitors(): Promise<any> {
+		throw new Error('Method not implemented.')
+	}
+}

shared/packages/worker/src/worker/accessorHandlers/__tests__/localFolder.spec.ts

@@ -0,0 +1,79 @@
+import {
+	AccessorOnPackage,
+	protectString,
+	setupLogger,
+	initializeLogger,
+	ProcessConfig,
+	Expectation,
+	Accessor,
+} from '@sofie-package-manager/api'
+import { Content, LocalFolderAccessorHandle } from '../localFolder'
+import { PassiveTestWorker } from './lib'
+
+const processConfig: ProcessConfig = {
+	logPath: undefined,
+	logLevel: undefined,
+	unsafeSSL: false,
+	certificates: [],
+}
+initializeLogger({ process: processConfig })
+test('checkHandleBasic', () => {
+	const logger = setupLogger(
+		{
+			process: processConfig,
+		},
+		''
+	)
+	const worker = new PassiveTestWorker(logger)
+
+	function getLocalFolderAccessor(
+		accessor: AccessorOnPackage.LocalFolder,
+		content: Content,
+		workOptions: Expectation.WorkOptions.Base &
+			Expectation.WorkOptions.RemoveDelay &
+			Expectation.WorkOptions.UseTemporaryFilePath = {}
+	) {
+		accessor.type = Accessor.AccessType.LOCAL_FOLDER
+		return new LocalFolderAccessorHandle(worker, protectString('local0'), accessor, content, workOptions)
+	}
+
+	expect(() => getLocalFolderAccessor({}, {}).checkHandleBasic()).toThrowError('Bad input data')
+
+	// missing accessor.folderPath:
+	expect(getLocalFolderAccessor({}, { filePath: 'amb.amp4' }).checkHandleBasic()).toMatchObject({
+		success: false,
+		reason: { tech: 'Folder path not set' },
+	})
+
+	// All OK:
+	expect(getLocalFolderAccessor({ folderPath: '/a/b/c' }, { filePath: 'amb.amp4' }).checkHandleBasic()).toMatchObject(
+		{
+			success: true,
+		}
+	)
+
+	// Absolute file path:
+	expect(
+		getLocalFolderAccessor({ folderPath: '/base/media' }, { filePath: '//secret/amb.amp4' }).checkHandleBasic()
+	).toMatchObject({
+		success: false,
+		reason: { tech: expect.stringMatching(/File path.*absolute path/) },
+	})
+	expect(
+		getLocalFolderAccessor({ folderPath: 'D:\\media' }, { filePath: 'C:\\secret\\amb.amp4' }).checkHandleBasic()
+	).toMatchObject({
+		success: false,
+		reason: { tech: expect.stringMatching(/File path.*absolute path/) },
+	})
+
+	// File path outside of folder path:
+	expect(
+		getLocalFolderAccessor({ folderPath: '/base/media' }, { filePath: '../secret/amb.amp4' }).checkHandleBasic()
+	).toMatchObject({
+		success: false,
+		reason: {
+			user: `File path is outside of folder path`,
+			tech: expect.stringMatching(/Full path.*does not start with/),
+		},
+	})
+})