[temp] to compare with pr

[mihainradu]

This is how are pull requests handled by FreeRDP

1. Every new pull request needs to build and pass the unit tests at https://ci.freerdp.com
2. At least 1 (better two) people need to review and test a pull request and agree to accept

Preparations before creating a pull

• Rebase your branch to current master, no merges allowed!
• Try to clean up your commit history, group changes to commits
• Check your formatting! A clang-format script can be found at .clang-format
  • The cmake target clangformat reformats the whole codebase
• Optional (but higly recommended)
  • Run a clang scanbuild before and after your changes to avoid introducing new bugs
  • Run your compiler at pedantic level to check for new warnings

To ease accepting your contribution

• Give the pull request a proper name so people looking at it have an basic idea what it is for
• Add at least a brief description what it does (or should do :) and what it's good for
• Give instructions on how to test your changes
• Ideally add unit tests if adding new features

What you should be prepared for

• fix issues found during the review phase
• Joining IRC #freerdp to talk to other developers or help them test your pull might accelerate acceptance
• Joining our mailing list freerdp-devel@lists.sourceforge.net may be helpful too.

Please remove this text before submitting your pull!

To fix this issue, the permissions key should be added to the job or to the top level of the workflow file to explicitly specify the minimum required permissions for running the workflow. The minimal necessary permissions for most CodeQL-only analysis workflows are contents: read (enough to read the repository contents and run analysis, not enough to write changes or interact elsewhere). This should be inserted either at the root level, or directly under the job that requires it; in this case, following the flagged issue on line 25, the block should be added under the analyze job in the .github/workflows/codeql-analysis.yml file. No new imports, methods, or other variable changes are required.

---

How to, in general terms, fix the problem:
Avoid logging sensitive information such as passwords in cleartext. When logging command invocations or arguments that potentially contain sensitive information, the password should be redacted, masked, or replaced with a safe placeholder before logging. This can be done by parsing and removing/replacing the sensitive portion in any logged string, based on knowledge of which commands/parameters may include it.

Best way to fix without changing existing functionality:

• In ProcessRunner.Run, ensure that logging methods (LogRunStarted, LogRunWaitCanceled, LogRunSucceeded, LogRunFailed) do not receive sensitive values unredacted in the arguments field.
• Implement a logic to redact passwords in commands known to include literal passwords, such as when calling net user ... [password] ... or similar commands as seen in the data flow.
• Add a utility method in ProcessRunner to identify and mask passwords in command arguments (e.g., replace password with "****" when logging).
• Use the redacted command arguments in all logger invocations within this method.

What is needed:

• A static method or private helper in ProcessRunner to redact passwords from argument strings based on the command (likely a regular expression for net user ... [password]).
• Use this helper to create a redacted version of startInfo.Arguments for all logger calls.
• There is no need for third-party dependencies—.NET regexes are sufficient.
• No changes to the business logic of how arguments are passed to the process: the actual command launched is unchanged; only logs are sanitized.

Files/lines to change:

• In UiPath.FreeRdpClient/UiPath.SessionTools/Processes/ProcessRunner.cs, update every place startInfo.Arguments is passed to _logger?.Log* within Run.
• Add the redaction helper as a private static method to ProcessRunner.

---

To prevent accidental storage of sensitive information, especially passwords, in clear text logs, the logging statements that include startInfo.Arguments need to be sanitized before being logged.

• The affected regions are wherever _logger?.LogRunWaitCanceled, _logger?.LogRunStarted, _logger?.LogRunFailed, and _logger?.LogRunSucceeded are called, passing startInfo.Arguments.
• The recommended fix is to mask any sensitive information in startInfo.Arguments before logging it. For the currently known use case (the net user ... password ... command), password values are space-delimited in the arguments. We can implement a utility method that scans command lines for likely passwords and replaces them with a mask (e.g., *****).
• Specifically:
  • Add a method SanitizeArgumentsForLogging(string arguments) that detects and masks passwords in command strings; call this method when passing startInfo.Arguments as a log parameter.
  • Only edit code within ProcessRunner.cs as that's where the logging happens.
  • Ensure the masking function is straightforward, deterministic, and does not break normal logging for non-sensitive usages.

---

To remediate this issue, we need to ensure that sensitive information—specifically passwords included in command arguments—is not written in cleartext to logs. The fix entails redacting passwords from the arguments string before passing it to any logging functions. This can be achieved by creating and using a helper method that detects password parameters in the command arguments and redacts their value, replacing them with a placeholder (e.g., "*****"). This method should be applied whenever process arguments are logged (e.g., in calls to _logger?.LogRunStarted(...), _logger?.LogRunSucceeded(...), _logger?.LogRunFailed(...), and _logger?.LogRunWaitCanceled(...)). Only the logged value should be redacted; the value used to execute the process must remain unmodified so functionality is unchanged.

Concretely:

• Add a method to redact password values from argument strings.
• Use this method to sanitize startInfo.Arguments before logging, e.g., RedactSensitiveArguments(startInfo.Arguments).
• Update all logger invocations in ProcessRunner.Run (lines 42, 53, 60, 66) to use the redacted version.

Depending on the exact command grammar, basic redaction could, for example, replace value after a flag or positional password. For demonstration, we'll apply a simple regular expression to mask sequences that look like passwords in the common command constructions shown.

---

Generally, to remediate "clear text storage of sensitive information", we should avoid logging secret values. In this context, the arguments field constructed for process invocation may contain passwords in clear text (notably for commands like net user ... password), but logging infrastructure currently logs the entire arguments string.

A robust fix is to sanitize or redact sensitive information (such as passwords) before logging. This means masking or removing any passwords from arguments before passing it to the logger. A helper function should be added to redact passwords from such parameter strings. This helper can use a regular expression or simple text replacement to replace passwords in "net user" commands by replacing the password argument with a constant value like "REDACTED". All logging calls using startInfo.Arguments should instead use a sanitized version (e.g., RedactSensitiveArguments(startInfo.FileName, startInfo.Arguments)).

This requires:

• Adding a method, e.g., RedactSensitiveArguments(string fileName, string arguments), to perform redaction.
• Replacing all logger calls that use startInfo.Arguments with sanitized arguments.
• Importing System.Text.RegularExpressions for regex if needed.

All changes should be performed in UiPath.FreeRdpClient/UiPath.SessionTools/Processes/ProcessRunner.cs, as that is where the logging (and the original problem) occurs.

---

The problem is on line 113, where pointer arithmetic is performed on a char**, which automatically scales by sizeof(char*), but the code multiplies the offset by an additional sizeof(char*), effectively causing a miscalculation. The intention seems to be to compute a pointer to a buffer region after the array of pointers. This should be done by first casting the pointer to a char*, so arithmetic operates in bytes. The correct way is:

• Calculate the base address for the buffer portion by taking the start address: (char*)str + ctx->pos * sizeof(char*)
• Add the offset for the current message: x * UNWIND_MAX_LINE_SIZE

So, the fixed line is:

char* msg = ((char*)str) + ctx->pos * sizeof(char*) + x * UNWIND_MAX_LINE_SIZE;

Only line 113 needs this fix, with no extra includes or definitions.

---