====== Security Guidelines ====== Mandatory security standards for WvdS FPC RAD Suite, compliant with KRITIS/NIS2. These guidelines are **non-negotiable**. Code that violates them will not be accepted. ===== OWASP Top 10 Prevention ===== ==== CWE-78: Command Injection ==== **Risk:** Execution of arbitrary commands through manipulated input. // FORBIDDEN - Insecure! Exec('cmd /c ' + UserInput); Shell('fpc ' + ProjectPath + ' ' + UserArgs); // CORRECT - Parameterized Options.Shell := False; Args := TStringArray.Create; Args.Add(ProjectPath); Spawn('fpc', Args, Options); **Measures:** * Always use ''shell: false'' * Pass arguments as array * Validate paths before use ==== CWE-22: Path Traversal ==== **Risk:** Access to files outside the allowed directory. // FORBIDDEN - Insecure! FileName := BasePath + UserInput; ReadFile(FileName); // CORRECT - Validation function IsPathSafe(const ABasePath, AUserInput: string): Boolean; var ResolvedPath: string; begin // Do not allow .. if Pos('..', AUserInput) > 0 then Exit(False); ResolvedPath := ExpandFileName(Concat(ABasePath, AUserInput)); Result := Pos(ABasePath, ResolvedPath) = 1; end; if IsPathSafe(BasePath, UserInput) then ReadFile(Concat(BasePath, UserInput)); **Measures:** * Block ''..'' sequences * Normalize and validate paths * Base path must be prefix of result path ==== CWE-20: Input Validation ==== **Risk:** Invalid data leads to malfunction or attack. // FORBIDDEN - No validation procedure CreateProject(const AName: string); begin MakeDir(AName); // What if AName = '../../../etc'? end; // CORRECT - Validation function ValidateProjectName(const AName: string; out AError: string): Boolean; const ALLOWED_CHARS = ['a'..'z', 'A'..'Z', '0'..'9', '_', '-']; MAX_LENGTH = 64; var I: Integer; begin Result := False; if AName = '' then begin AError := rsProjectNameEmpty; Exit; end; if Length(AName) > MAX_LENGTH then begin AError := Format(rsProjectNameTooLong, [MAX_LENGTH]); Exit; end; for I := 1 to Length(AName) do if not (AName[I] in ALLOWED_CHARS) then begin AError := Format(rsProjectNameInvalidChar, [AName[I]]); Exit; end; Result := True; end; ==== CWE-316: Cleartext Credentials ==== **Risk:** Credentials in code or logs. // FORBIDDEN - Credentials in code const API_TOKEN = 'ghp_xxxxxxxxxxxx'; DB_PASSWORD = 'secret123'; // FORBIDDEN - Logging credentials LogDebug('Token: %s', [Token]); // CORRECT - Environment Variables Token := GetEnvironmentVariable('GITHUB_TOKEN'); if Token = '' then raise EWvdSConfigError.Create(rsTokenNotConfigured); // CORRECT - Masked logging LogDebug('Token configured: %s', [BoolToStr(Token <> '', True)]); ==== CWE-532: Log Injection ==== **Risk:** Sensitive data visible in logs. // FORBIDDEN LogInfo('User login: %s with password: %s', [User, Password]); LogDebug('API response: %s', [FullResponse]); // May contain tokens! // CORRECT LogInfo('User login: %s', [User]); // No password LogDebug('API response received, length: %d', [Length(Response)]); ==== CWE-79: Cross-Site Scripting (XSS) ==== **Risk:** Injection of script code into WebViews. // FORBIDDEN - Unescaped HTML WebView.Html := '
' + UserInput + '
'; // CORRECT - Escaping function EscapeHtml(const AText: string): string; begin Result := AText; Result := StringReplace(Result, '&', '&', [rfReplaceAll]); Result := StringReplace(Result, '<', '<', [rfReplaceAll]); Result := StringReplace(Result, '>', '>', [rfReplaceAll]); Result := StringReplace(Result, '"', '"', [rfReplaceAll]); Result := StringReplace(Result, '''', ''', [rfReplaceAll]); end; WebView.Html := '
' + EscapeHtml(UserInput) + '
';
==== CWE-20: WebView Message Whitelist ==== **Risk:** Unknown message types enable attacks on WebView handlers. Every WebView message handler MUST implement a whitelist of allowed message types. // FORBIDDEN - No message type check procedure HandleMessage(AMessage: TJSObject); var MsgType: string; begin MsgType := string(AMessage['type']); case MsgType of 'browse': HandleBrowse(AMessage); 'save': HandleSave(AMessage); end; end; // CORRECT - With whitelist const ALLOWED_MESSAGE_TYPES: array[0..6] of string = ( 'browse', 'save', 'cancel', 'validatePath', 'autoDetect', 'config', 'pathSelected' ); function IsAllowedMessageType(const AType: string): Boolean; var I: Integer; begin Result := False; for I := Low(ALLOWED_MESSAGE_TYPES) to High(ALLOWED_MESSAGE_TYPES) do if ALLOWED_MESSAGE_TYPES[I] = AType then Exit(True); end; procedure HandleMessage(AMessage: TJSObject); var MsgType: string; begin MsgType := string(AMessage['type']); // Whitelist check FIRST if not IsAllowedMessageType(MsgType) then begin LogWarning(rsUnknownMessageType, [MsgType]); Exit; end; case MsgType of 'browse': HandleBrowse(AMessage); 'save': HandleSave(AMessage); // ... end; end; **Measures:** * Define whitelist with all allowed message types * Log and reject unknown types * Update whitelist when adding new features ==== Defensive Node.js API Calls ==== **Risk:** Node.js APIs can have different methods depending on context. Practical problem: ''stdout.setEncoding'' is not a function when process is not correctly initialized. // FORBIDDEN - Direct method calls without check procedure SetupProcessHandlers; begin asm this.FProcess.stdout.setEncoding('utf8'); this.FProcess.stderr.setEncoding('utf8'); end; end; // CORRECT - Defensive typeof checks procedure SetupProcessHandlers; begin asm if (this.FProcess && this.FProcess.stdout) { if (typeof this.FProcess.stdout.setEncoding === 'function') { this.FProcess.stdout.setEncoding('utf8'); } if (typeof this.FProcess.stdout.on === 'function') { this.FProcess.stdout.on('data', this.HandleStdout.bind(this)); } } if (this.FProcess && this.FProcess.stderr) { if (typeof this.FProcess.stderr.setEncoding === 'function') { this.FProcess.stderr.setEncoding('utf8'); } if (typeof this.FProcess.stderr.on === 'function') { this.FProcess.stderr.on('data', this.HandleStderr.bind(this)); } } end; end; **Measures:** * ''typeof ... === 'function''' before every method call * Check object existence (''if (obj && obj.property)'') * Handle missing APIs gracefully ===== Error Handling ===== ==== No Empty Exception Handlers ==== // FORBIDDEN try DoSomething; except // Swallowing errors end; // CORRECT try DoSomething; except on E: ESpecificError do begin LogError(rsSpecificError, [E.Message]); // Handle... end; on E: Exception do begin LogError(rsUnexpectedError, [E.ClassName, E.Message]); raise; // Or handle appropriately end; end; ==== Specific Exception Types ==== // CORRECT - Specific exceptions try Result := DoOperation; except on E: EFileNotFoundException do HandleFileNotFound(E.FileName); on E: EAccessDenied do HandleAccessDenied(E.Path); on E: ENetworkError do HandleNetworkError(E.Url); on E: Exception do HandleUnexpectedError(E); end; ===== Debug Logging ===== Debug logging requires **two conditions**: - **Compile-time:** ''{$IFDEF DEBUG}'' - **Runtime:** ''--debug'' parameter {$IFDEF DEBUG} var DebugLogFile: TextFile; DebugEnabled: Boolean; procedure InitDebugLogging; begin DebugEnabled := ParamStr(1) = '--debug'; if DebugEnabled then begin AssignFile(DebugLogFile, Format('debug-%s.log', [FormatDateTime('yymmddhhnnss', Now)])); Rewrite(DebugLogFile); end; end; procedure LogDebugTrace(const AMessage: string; const AArgs: array of const); begin if not DebugEnabled then Exit; WriteLn(DebugLogFile, Format('[%s] %s', [FormatDateTime('hh:nn:ss.zzz', Now), Format(AMessage, AArgs)])); Flush(DebugLogFile); end; {$ENDIF} ==== What May Be Logged ==== ^ Allowed ^ Forbidden ^ | Filenames, paths | Tokens, API keys | | Action names | Passwords | | Numeric IDs | Session IDs | | Error messages | Full requests/responses | | Configuration keys | Configuration values (sensitive) | ===== Secure Defaults ===== // Secure default values const DEFAULT_TIMEOUT = 30000; // 30 seconds MAX_FILE_SIZE = 10 * 1024 * 1024; // 10 MB MAX_RECURSION_DEPTH = 100; type TSpawnOptions = record Shell: Boolean; // Default: False (secure) Timeout: Integer; // Default: 30000 WorkingDir: string; Environment: TStringArray; end; function DefaultSpawnOptions: TSpawnOptions; begin Result.Shell := False; // IMPORTANT: No shell! Result.Timeout := DEFAULT_TIMEOUT; Result.WorkingDir := ''; Result.Environment := nil; end; ===== Code Review Checklist ===== Before merge MUST be checked: SECURITY: [ ] No hardcoded credentials [ ] All user inputs validated [ ] No shell injection possible [ ] No path traversal possible [ ] HTML is escaped in WebViews [ ] No sensitive data in logs ERROR HANDLING: [ ] No empty exception handlers [ ] Specific exceptions handled [ ] Errors are logged (without sensitive data) CONFIGURATION: [ ] Secure defaults used [ ] Timeouts defined [ ] Limits defined (file size, recursion) ===== Security Audit ==== Regular checking with automated tools: ==== wvds-lint Security Checks ==== wvds-lint security --path sources/ Checks: * Exec/Shell calls * Path operations without validation * Hardcoded strings (potential credentials) * Empty exception handlers * Log calls with sensitive parameters ==== Manual Review ==== For every new extension: * Check all entry points * Trace all external inputs * Review WebView communication ===== Incident Response ===== When a security vulnerability is discovered: - **Do not publish** until patch is ready - Issue on GitHub with label "security" (private) - Develop and test patch - Release new version - Publish advisory ===== See Also ===== * [[.:code-konventionen|Code Conventions]] * [[.:debugging|Debugging]] * [[.:testing|Testing]] * [[https://owasp.org/www-project-top-ten/|OWASP Top 10]]