You've already forked wc-licensed-product-client
Fix security vulnerabilities identified in audit
- Add JSON encoding error handling in ResponseSignature to prevent silent failures - Sanitize exception messages to prevent information disclosure - Fix header normalization to treat empty values as null - Add SSRF protection with URL validation and private IP blocking - Replace custom key derivation with RFC 5869 compliant hash_hkdf() - Add input validation in DTO fromArray() methods - Add DateTime exception handling in DTOs Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
@@ -33,6 +33,16 @@ final class SecureLicenseClient implements LicenseClientInterface
|
||||
{
|
||||
private const CACHE_TTL = 300;
|
||||
|
||||
/** @var string[] Private IPv4 ranges (CIDR notation) */
|
||||
private const PRIVATE_IP_RANGES = [
|
||||
'10.0.0.0/8',
|
||||
'172.16.0.0/12',
|
||||
'192.168.0.0/16',
|
||||
'127.0.0.0/8',
|
||||
'169.254.0.0/16',
|
||||
'0.0.0.0/8',
|
||||
];
|
||||
|
||||
private readonly LoggerInterface $logger;
|
||||
private readonly StringEncoder $encoder;
|
||||
private readonly string $apiPath;
|
||||
@@ -52,10 +62,12 @@ final class SecureLicenseClient implements LicenseClientInterface
|
||||
private readonly int $cacheTtl = self::CACHE_TTL,
|
||||
private readonly bool $verifyIntegrity = false,
|
||||
?StringEncoder $encoder = null,
|
||||
bool $allowInsecureHttp = false,
|
||||
) {
|
||||
$this->logger = $logger ?? new NullLogger();
|
||||
$this->encoder = $encoder ?? new StringEncoder();
|
||||
$this->apiPath = $this->encoder->decode(self::ENCODED_API_PATH);
|
||||
$this->validateBaseUrl($baseUrl, $allowInsecureHttp);
|
||||
|
||||
if ($this->verifyIntegrity) {
|
||||
$this->checkIntegrity();
|
||||
@@ -199,13 +211,15 @@ final class SecureLicenseClient implements LicenseClientInterface
|
||||
} catch (LicenseException | SignatureException $e) {
|
||||
throw $e;
|
||||
} catch (\Throwable $e) {
|
||||
// Log full error for debugging but sanitize for user-facing message
|
||||
$this->logger->error('License API request failed', [
|
||||
'endpoint' => $endpoint,
|
||||
'error' => $e->getMessage(),
|
||||
'exception_class' => $e::class,
|
||||
'error_code' => $e->getCode(),
|
||||
]);
|
||||
|
||||
throw new LicenseException(
|
||||
'Failed to communicate with license server: ' . $e->getMessage(),
|
||||
'Failed to communicate with license server',
|
||||
null,
|
||||
0,
|
||||
$e
|
||||
@@ -239,7 +253,13 @@ final class SecureLicenseClient implements LicenseClientInterface
|
||||
|
||||
foreach ($headers as $name => $values) {
|
||||
// HTTP client returns arrays of values; take the first one
|
||||
$normalized[$name] = is_array($values) ? ($values[0] ?? '') : $values;
|
||||
// Empty arrays or empty strings should be treated as missing (null)
|
||||
if (is_array($values)) {
|
||||
$value = $values[0] ?? null;
|
||||
$normalized[$name] = ($value !== '' && $value !== null) ? $value : null;
|
||||
} else {
|
||||
$normalized[$name] = ($values !== '' && $values !== null) ? $values : null;
|
||||
}
|
||||
}
|
||||
|
||||
return $normalized;
|
||||
@@ -298,4 +318,69 @@ final class SecureLicenseClient implements LicenseClientInterface
|
||||
}
|
||||
return $key;
|
||||
}
|
||||
|
||||
/**
|
||||
* Validate the base URL to prevent SSRF attacks.
|
||||
*
|
||||
* @throws \InvalidArgumentException If the URL is invalid or potentially dangerous
|
||||
*/
|
||||
private function validateBaseUrl(string $url, bool $allowInsecureHttp): void
|
||||
{
|
||||
if ($url === '') {
|
||||
throw new \InvalidArgumentException('Base URL cannot be empty');
|
||||
}
|
||||
|
||||
$parsed = parse_url($url);
|
||||
if ($parsed === false || !isset($parsed['scheme'], $parsed['host'])) {
|
||||
throw new \InvalidArgumentException('Invalid base URL format');
|
||||
}
|
||||
|
||||
$scheme = strtolower($parsed['scheme']);
|
||||
$host = strtolower($parsed['host']);
|
||||
|
||||
// Require HTTPS unless explicitly allowed for localhost
|
||||
if ($scheme !== 'https') {
|
||||
if ($scheme !== 'http') {
|
||||
throw new \InvalidArgumentException('Base URL must use HTTP or HTTPS scheme');
|
||||
}
|
||||
$isLocalhost = $host === 'localhost' || $host === '127.0.0.1';
|
||||
if (!$allowInsecureHttp && !$isLocalhost) {
|
||||
throw new \InvalidArgumentException(
|
||||
'Base URL must use HTTPS for non-localhost hosts. ' .
|
||||
'Set allowInsecureHttp=true to allow HTTP (not recommended for production).'
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
// Resolve hostname and check for private IPs
|
||||
$ip = gethostbyname($host);
|
||||
if ($ip !== $host && $this->isPrivateIp($ip)) {
|
||||
throw new \InvalidArgumentException(
|
||||
'Base URL resolves to a private IP address, which is not allowed'
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Check if an IP address is in a private range.
|
||||
*/
|
||||
private function isPrivateIp(string $ip): bool
|
||||
{
|
||||
$ipLong = ip2long($ip);
|
||||
if ($ipLong === false) {
|
||||
return false;
|
||||
}
|
||||
|
||||
foreach (self::PRIVATE_IP_RANGES as $range) {
|
||||
[$subnet, $bits] = explode('/', $range);
|
||||
$subnetLong = ip2long($subnet);
|
||||
$mask = -1 << (32 - (int) $bits);
|
||||
|
||||
if (($ipLong & $mask) === ($subnetLong & $mask)) {
|
||||
return true;
|
||||
}
|
||||
}
|
||||
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user