[PATCH] readconf: reject 'all' combined with other Match attributes

[Muhammad Bilal]

match_cfg_line() (readconf.c) is supposed to reject a Match line such
as 'Match user foo all', since the 'all' criterion must appear alone.
The check is 'if (attributes > 1 || ...)', but 'attributes' was only
incremented *after* the 'all' branch returns, so by the time a
trailing 'all' token was checked, 'attributes' still held the count
from before that token -- one less than it should have been. A line
with exactly one prior attribute (e.g. 'user foo') then passed the
'attributes > 1' test and was silently accepted.

sshd's equivalent function, process_server_match_line() in
servconf.c, increments its 'attributes' counter before the 'all'
check (servconf.c:842-845) and correctly rejects the same line. This
moves readconf.c's 'attributes++;' to the same position, ahead of the
'all' check, matching servconf.c.

Verified against a live build: 'ssh -F <config> -G host' on a config
containing 'Match user foo all' exited 0 and printed the resolved
config before this change; after the change it exits 255 with
"'all' cannot be combined with other Match attributes", matching
'sshd -T' on the equivalent sshd_config line. A plain 'Match all' on
its own line still parses and applies correctly, confirming no
regression for the common case.
---
 readconf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/readconf.c b/readconf.c
index bc48b4f55..6d17b49f7 100644
--- a/readconf.c
+++ b/readconf.c
@@ -734,6 +734,7 @@ match_cfg_line(Options *options, const char *full_line, int *acp, char ***avp,
 		this_result = 1;
 		if ((negate = (attrib[0] == '!')))
 			attrib++;
+		attributes++;
 		/* Criterion "all" has no argument and must appear alone */
 		if (strcasecmp(attrib, "all") == 0) {
 			if (attributes > 1 ||
@@ -751,7 +752,6 @@ match_cfg_line(Options *options, const char *full_line, int *acp, char ***avp,
 				result = negate ? 0 : 1;
 			goto out;
 		}
-		attributes++;
 		/* criteria "final" and "canonical" have no argument */
 		if (strcasecmp(attrib, "canonical") == 0 ||
 		    strcasecmp(attrib, "final") == 0) {
-- 
2.43.0