Skip to content

wolfsshd: reject Match blocks using unimplemented selectors#1026

Open
yosuke-wolfssl wants to merge 1 commit into
wolfSSL:masterfrom
yosuke-wolfssl:fix/f_3877
Open

wolfsshd: reject Match blocks using unimplemented selectors#1026
yosuke-wolfssl wants to merge 1 commit into
wolfSSL:masterfrom
yosuke-wolfssl:fix/f_3877

Conversation

@yosuke-wolfssl

Copy link
Copy Markdown
Contributor

Summary

wolfsshd silently ignored Match blocks keyed on any selector other than User/Group.

When HandleMatch parsed a Match directive it only called AddRestrictedCase for the User and Group selectors. A block keyed on Address, Host, LocalAddress, LocalPort, or RDomain left both usrAppliesTo and groupAppliesTo NULL, yet ParseConfigLine still returned WS_SUCCESS. Because wolfSSHD_GetUserConf only matches a node when one of those pointers is non-NULL, the block was unreachable and the global config was used instead.

This is a fail-open misconfiguration: an administrator who tightens authentication for external networks with, e.g., Match Address 10.0.0.0/8 gets no error or warning, but the rules never apply.

Fix

Reject Match blocks that contain no supported selector instead of accepting them silently.

In HandleMatch, after attempting to parse User/Group, if both selectors are unresolved the directive is rejected with WS_FATAL_ERROR and a clear log message:

[SSHD] Unsupported Match selector, only User and Group are handled

The guard fires only when neither selector resolved, so all supported forms still work:

  • Match User <name> (User-only)
  • Match Group <name> (Group-only)
  • Match User <name> Group <name> (both)

The orphaned copied config node (never linked into the list on the error path) is freed; this is leak- and double-free-safe since wolfSSHD_ConfigCopy zero-initializes the node and does not copy next.

Testing

Added test_MatchUnsupportedSelector in apps/wolfsshd/test/test_configuration.c with 9 vectors:

  • Accepted: Match User, Match Group
  • Rejected (named selectors): Match Address, Match Host, Match LocalAddress, Match LocalPort, Match RDomain
  • Rejected (no-selector forms): Match all, bare Match

All scenarios pass. Built and ran the suite under ASan + UBSan with no sanitizer reports.

Files changed

  • apps/wolfsshd/configuration.c — reject Match blocks with no supported selector; free orphaned node on error path
  • apps/wolfsshd/test/test_configuration.c — add test_MatchUnsupportedSelector

@yosuke-wolfssl yosuke-wolfssl self-assigned this Jun 15, 2026
Copilot AI review requested due to automatic review settings June 15, 2026 02:50

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR hardens wolfsshd configuration parsing by preventing Match blocks that can never be applied (because they use only unimplemented selectors) from being accepted silently, avoiding a fail-open misconfiguration scenario.

Changes:

  • Reject Match directives that resolve neither User nor Group, logging an explicit error and returning WS_FATAL_ERROR.
  • Free the newly-copied (but unlinked) config node on the Match failure path to avoid leaks.
  • Add a unit test covering accepted (User/Group) and rejected (unsupported / no-selector) Match forms.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
apps/wolfsshd/configuration.c Adds rejection + cleanup for Match directives that contain no supported selector (User/Group).
apps/wolfsshd/test/test_configuration.c Adds unit coverage to ensure unsupported/no-selector Match directives are rejected.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread apps/wolfsshd/configuration.c Outdated

@wolfSSL-Fenrir-bot wolfSSL-Fenrir-bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fenrir Automated Review — PR #1026

Scan targets checked: none
Failed targets: wolfssh-bugs, wolfssh-src

⚠️ Review incomplete — one or more scan targets failed before findings could be produced. See the Fenrir PR review detail page for logs.

@wolfSSL-Fenrir-bot wolfSSL-Fenrir-bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fenrir Automated Review — PR #1026

Scan targets checked: none
Failed targets: wolfssh-bugs, wolfssh-src

⚠️ Review incomplete — one or more scan targets failed before findings could be produced. See the Fenrir PR review detail page for logs.

@wolfSSL-Fenrir-bot wolfSSL-Fenrir-bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fenrir Automated Review — PR #1026

Scan targets checked: wolfssh-bugs, wolfssh-src

No new issues found in the changed files. ✅

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.

4 participants