Fix secure linter warnings (#457)

* fix object injection sinks (disable warning for test code)

Signed-off-by: Jens Reinecke <[email protected]>

* use of variables in fs functions

Signed-off-by: Jens Reinecke <[email protected]>

---------

Signed-off-by: Jens Reinecke <[email protected]>

Author: jreineckearm

src/__test__/test-data-factory.ts

@@ -35,6 +35,8 @@ export function makeFactory<T extends object>(initializer: Initializer<T>): Fact
         const result = { ...options } as Mutable<T>;
         for (const key in initializer) {
             if (!(key in result)) {
+                // disable rule for next line, not used in production code
+                // eslint-disable-next-line security/detect-object-injection
                 result[key] = initializer[key].call(result, result);
             }
         }

src/cbuild-run/cbuild-run-reader.ts

@@ -15,30 +15,29 @@
  */
 
 import * as yaml from 'yaml';
-import { CbuildRunType } from './cbuild-run-types';
+import { CbuildRunRootType, CbuildRunType } from './cbuild-run-types';
 import { FileReader, VscodeFileReader } from '../desktop/file-reader';
 import { getCmsisPackRootPath } from '../utils';
 
-const ROOT_NODE = 'cbuild-run';
 const CMSIS_PACK_ROOT_ENVVAR = '${CMSIS_PACK_ROOT}';
 
 export class CbuildRunReader {
-    private cbuildRun?: CbuildRunType;
+    private cbuildRun: CbuildRunType | undefined;
 
     constructor(private reader: FileReader = new VscodeFileReader()) {}
 
     public hasContents(): boolean {
         return !!this.cbuildRun;
     }
 
-    public getContents(): CbuildRunType|undefined {
+    public getContents(): CbuildRunType | undefined {
         return this.cbuildRun;
     }
 
     public async parse(filePath: string): Promise<void> {
         const fileContents = await this.reader.readFileToString(filePath);
-        const fileRoot = yaml.parse(fileContents);
-        this.cbuildRun = fileRoot ? fileRoot[ROOT_NODE] : undefined;
+        const fileRoot = yaml.parse(fileContents) as CbuildRunRootType;
+        this.cbuildRun = fileRoot ? fileRoot['cbuild-run'] : undefined;
         if (!this.cbuildRun) {
             throw new Error(`Invalid '*.cbuild-run.yml' file: ${filePath}`);
         }

src/cbuild-run/cbuild-run-types.ts

@@ -173,3 +173,7 @@ export interface CbuildRunType {
     programming?: ProgrammingType[];
     'debug-topology'?: DebugTopologyType;
 };
+
+export interface CbuildRunRootType {
+    'cbuild-run': CbuildRunType;
+};

src/debug-configuration/gdbtarget-configuration-provider.ts

@@ -71,10 +71,18 @@ export class GDBTargetConfigurationProvider implements vscode.DebugConfiguration
         logger.debug(`\t${resolvedGDBConfig.target?.server} ${resolvedGDBConfig.target?.serverParameters?.join(' ')}`);
     }
 
+    private hasResolverFunction(resolverType: ResolverType, provider: vscode.DebugConfigurationProvider): boolean {
+        switch (resolverType) {
+            case 'resolveDebugConfiguration':
+                return !!provider.resolveDebugConfiguration;
+            case 'resolveDebugConfigurationWithSubstitutedVariables':
+                return !!provider.resolveDebugConfigurationWithSubstitutedVariables;
+        }
+    }
+
     private isRelevantSubprovider(resolverType: ResolverType, serverType: string, subProvider: GDBTargetConfigurationSubProvider): boolean {
         const serverTypeMatch = subProvider.serverRegExp.test(serverType);
-        const hasResolverFunction = !!subProvider.provider[resolverType];
-        return serverTypeMatch && hasResolverFunction;
+        return serverTypeMatch && this.hasResolverFunction(resolverType, subProvider.provider);
     }
 
     private getRelevantSubproviders(resolverType: ResolverType, serverType?: string): GDBTargetConfigurationSubProvider[] {
@@ -96,7 +104,7 @@ export class GDBTargetConfigurationProvider implements vscode.DebugConfiguration
             subproviders.forEach((subprovider, index) => logger.warn(`#${index}: '${subprovider.serverRegExp}'`));
         }
         const relevantProvider = subproviders[0];
-        if (!relevantProvider.provider[resolverType]) {
+        if (!this.hasResolverFunction(resolverType, relevantProvider.provider)) {
             logger.debug(`${resolverType}: Subprovider '${relevantProvider.serverRegExp}' does not implement '${resolverType}'.`);
             return undefined;
         }
@@ -118,7 +126,9 @@ export class GDBTargetConfigurationProvider implements vscode.DebugConfiguration
             return debugConfiguration;
         }
         logger.debug(`${resolverType}: Resolve config with subprovider '${subprovider.serverRegExp}'`);
-        const resolvedConfig = await subprovider.provider[resolverType]!(folder, debugConfiguration, token);
+        const resolvedConfig = resolverType === 'resolveDebugConfiguration'
+            ? await subprovider.provider.resolveDebugConfiguration!(folder, debugConfiguration, token)
+            : await subprovider.provider.resolveDebugConfigurationWithSubstitutedVariables!(folder, debugConfiguration, token);
         if (!resolvedConfig) {
             logger.error(`${resolverType}: Resolving config failed with subprovider '${subprovider.serverRegExp}'`);
             return undefined;

src/debug-configuration/gdbtarget-configuration.ts

@@ -75,3 +75,8 @@ export interface GDBTargetConfiguration extends vscode.DebugConfiguration {
     target?: TargetConfiguration;
     cmsis?: CMSISConfiguration;
 };
+
+export interface ExtendedGDBTargetConfiguration extends GDBTargetConfiguration {
+    // For additional configuration items not part of GDBTargetConfiguration but known to other extensions
+    definitionPath?: string;  // Default SVD path setting for Peripheral Inspector
+};

src/debug-configuration/subproviders/base-configuration-provider.ts

@@ -15,12 +15,10 @@
  */
 
 import * as vscode from 'vscode';
-import { GDBTargetConfiguration } from '../gdbtarget-configuration';
+import { ExtendedGDBTargetConfiguration, GDBTargetConfiguration } from '../gdbtarget-configuration';
 import { CbuildRunReader } from '../../cbuild-run';
 import { logger } from '../../logger';
 
-const DEFAULT_SVD_SETTING_NAME = 'definitionPath';
-
 export abstract class BaseConfigurationProvider implements vscode.DebugConfigurationProvider {
     protected _cbuildRunReader?: CbuildRunReader;
 
@@ -54,14 +52,15 @@ export abstract class BaseConfigurationProvider implements vscode.DebugConfigura
     }
 
     protected resolveSvdFile(debugConfiguration: GDBTargetConfiguration) {
+        const extDebugConfig = debugConfiguration as ExtendedGDBTargetConfiguration;
         const cbuildRunFilePath = debugConfiguration.cmsis?.cbuildRunFile;
         // 'definitionPath' is current default name for SVD file settings in Eclipse CDT Cloud Peripheral Inspector.
-        if (debugConfiguration[DEFAULT_SVD_SETTING_NAME] || !cbuildRunFilePath?.length) {
+        if (extDebugConfig.definitionPath !== undefined || !cbuildRunFilePath?.length) {
             return;
         }
         const svdFilePaths = this.cbuildRunReader.getSvdFilePaths(debugConfiguration?.target?.environment?.CMSIS_PACK_ROOT);
         // Needs update when we better support multiple `debugger:` YAML nodes
-        debugConfiguration[DEFAULT_SVD_SETTING_NAME] = svdFilePaths[0];
+        extDebugConfig.definitionPath = svdFilePaths[0];
     }
 
     protected abstract resolveServerParameters(debugConfiguration: GDBTargetConfiguration): Promise<GDBTargetConfiguration>;

src/desktop/builtin-tool-path.test.ts

@@ -41,7 +41,9 @@ describe('BuiltinToolPath', () => {
     it('should return the correct path for a given tool', () => {
         const builtinToolPath = new BuiltinToolPath('tools/pyocd/pyocd');
 
+        // eslint-disable-next-line security/detect-non-literal-fs-filename
         fs.mkdirSync(`${testFolder}/tools/pyocd`, { recursive: true });
+        // eslint-disable-next-line security/detect-non-literal-fs-filename
         fs.writeFileSync(`${testFolder}/tools/pyocd/pyocd${TOOL_EXTENSION}`, '');
 
         const expected = vscode.Uri.file(`${testFolder}/tools/pyocd/pyocd${TOOL_EXTENSION}`);
@@ -58,7 +60,9 @@ describe('BuiltinToolPath', () => {
 
     it('should return the directory of the tool', () => {
         const builtinToolPath = new BuiltinToolPath('tools/pyocd/pyocd');
+        // eslint-disable-next-line security/detect-non-literal-fs-filename
         fs.mkdirSync(`${testFolder}/tools/pyocd`, { recursive: true });
+        // eslint-disable-next-line security/detect-non-literal-fs-filename
         fs.writeFileSync(`${testFolder}/tools/pyocd/pyocd${TOOL_EXTENSION}`, '');
 
         const expected = vscode.Uri.file(`${testFolder}/tools/pyocd`);

src/desktop/builtin-tool-path.ts

@@ -29,8 +29,14 @@ export class BuiltinToolPath {
     public getAbsolutePath(): vscode.Uri | undefined {
         const extensionUri = vscode.extensions.getExtension(EXTENSION_ID)?.extensionUri;
         const absoluteUri = extensionUri?.with({ path: `${extensionUri.path}/${this.toolPath}${isWindows ? '.exe' : ''}` });
-        const fsPath = absoluteUri?.fsPath;
-        return (fsPath && fs.existsSync(fsPath)) ? absoluteUri : undefined;
+        if (!absoluteUri?.fsPath) {
+            return undefined;
+        }
+        const normalizedUri = vscode.Uri.file(path.normalize(absoluteUri.fsPath));
+        const normalizedFsPath = normalizedUri.fsPath;
+        // Cannot avoid it, but also no real user input here. `toolPath` only given programmatically.
+        // eslint-disable-next-line security/detect-non-literal-fs-filename
+        return fs.existsSync(normalizedFsPath) ? normalizedUri : undefined;
     }
 
     public getAbsolutePathDir(): string | undefined{