Skip to content

Fix UnicodeDecodeError in WinAdvFirewall on non-English Windows locales - #4300

Open
arielbosquez wants to merge 1 commit into
guardicore:developfrom
arielbosquez:fix/firewall-encoding-error
Open

Fix UnicodeDecodeError in WinAdvFirewall on non-English Windows locales#4300
arielbosquez wants to merge 1 commit into
guardicore:developfrom
arielbosquez:fix/firewall-encoding-error

Conversation

@arielbosquez

Copy link
Copy Markdown

What does this PR do?

WinAdvFirewall.is_enabled() calls subprocess.check_output("netsh advfirewall show currentprofile") and decodes the output with .decode() (defaulting to UTF-8). On Windows systems where the OEM code page is not UTF-8 (e.g. CP850 on Spanish-language Windows), netsh output containing accented characters causes an unhandled UnicodeDecodeError. This crashes the Agent's setup phase before it can begin scanning or propagating — the Agent completes initial recon and reports to the Island, but silently never proceeds past _setup().

Reproduced with:

This PR replaces netsh-based text parsing in is_enabled(), add_firewall_rule(), and remove_firewall_rule() with PowerShell's NetSecurity module (Get-NetFirewallProfile, New-NetFirewallRule, Remove-NetFirewallRule), reading results as structured JSON. PowerShell object property names are locale-independent, unlike netsh's human-readable output, so this fixes the crash for any non-English Windows locale, not just Spanish.

The legacy WinFirewall class (pre-Vista, unreachable on any currently-supported Windows version) received a minimal errors="ignore" safety patch on its own .decode() call, rather than a full rewrite, given its limited relevance today.

No related issue filed prior to this PR.

PR Checklist

  • Have you added an explanation of what your changes do and why you'd like to include them?
  • Is the TravisCI build passing?
  • Was the CHANGELOG.md updated to reflect the changes?
  • Was the documentation framework updated to reflect the changes?
  • Have you checked that you haven't introduced any duplicate code?

Testing Checklist

  • Added relevant unit tests?
  • Do all unit tests pass?
  • Do all end-to-end tests pass?
  • Any other testing performed?

    Tested by running a custom-built Agent (with this fix applied) against a Windows Server with OEM code page 850 (Spanish). Before the fix: Agent completed recon and reported to the Island, then crashed in _setup() with the UnicodeDecodeError above, every time, regardless of how it reached the target machine (SMB propagation, RDP, or manual execution). After the fix: Agent completed network scanning and propagation successfully via both SMB and SSH exploiters, with no errors. Also re-tested against an English-locale Windows Server to confirm no regression.

  • If applicable, add screenshots or log transcripts of the feature working

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant