F 739 : fix shell command injection

[miyazakh]

F-739 : Shell command injection via popen with unsensitized hostname
Add test coverage

Depend on : #211 (Fixed)
Depend on : #219

[Copilot]

Pull request overview

This PR addresses a shell command injection risk in the client hostname resolution path (when using popen("host ...")) and expands regression coverage to detect attempted command execution via crafted -connect hostnames.

Changes:

• Add hostname character validation before constructing a popen() command in the client.
• Add client regression tests that ensure injected shell metacharacters do not result in file creation.
• Loosen OCSP interop test error-message matching and adjust hash-type switch guards for newer wolfSSL versions.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

File	Description
src/client/client.c	Adds hostname validation to prevent shell metacharacters from reaching the popen() command.
tests/client/client-test.sh	Adds regression tests intended to detect hostname-based shell injection.
tests/ocsp/ocsp-interop-test.sh	Expands grep patterns for expected failure messages in interop testing.
src/x509/clu_x509_sign.c	Updates preprocessor version gating for additional WC_HASH_TYPE_* enum cases.

---

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

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.

---

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

[dgarske]

🐺 Skoll Code Review

Overall recommendation: APPROVE
Findings: 2 total — 2 posted, 0 skipped

Posted findings

• [Low] #include <ctype.h> placed between wolfSSL headers — src/client/client.c:37
• [Medium] Missing backtick command-substitution injection test case — tests/client/client-test.sh:28-60

Review generated by Skoll via openclaw

[dgarske]

🔵 [Low] #include <ctype.h> placed between wolfSSL headers
🔧 NIT convention

The new #include <ctype.h> is placed on line 37 between wolfssl/wolfcrypt/settings.h (line 35) and wolfssl/ssl.h (line 39). In wolfSSL convention, settings.h is typically followed immediately by other wolfSSL headers since it sets up preprocessor macros those headers depend on. System includes are conventionally grouped together. The include is needed (for the pre-existing isalpha() call on line 142), so adding it explicitly is correct — only the placement is slightly off.

Suggestion:

Suggested change

	#include <ctype.h>
	Move `#include <ctype.h>` to after `#include <wolfssl/ssl.h>` or group it with the other system includes near the top of the file (around the `#include <config.h>` block).

[dgarske]

🟡 [Medium] Missing backtick command-substitution injection test case
💡 SUGGEST test

The regression tests cover three shell injection vectors: semicolons (;), dollar-paren command substitution ($()), and pipes (|). However, backtick command substitution (`cmd`) is a fourth classic shell injection vector that is not tested. While the allowlist validation in client.c blocks backticks (they are not in [A-Za-z0-9.-]), adding a test for completeness would strengthen the regression suite and document the intent. An & (background execution) test would also be a nice-to-have.

Suggestion:

Suggested change

	# Regression tests: shell injection via hostname must not execute injected command.
	# Backtick command substitution: "`touch clu_injection_probe.txt`" passed as peer
	./wolfssl s_client -connect 'evil`touch clu_injection_probe.txt`.com:443' \
	2>/dev/null
	if [ -f "$INJFILE" ]; then
	echo "SECURITY FAILURE: command injection via hostname (backtick)"
	rm -f "$INJFILE"
	exit 99
	fi