Fix false positives in credential and XSS detection
- XSS: Fix script tag detection regex to not cross tag boundaries
Previously {!! !!} in HTML between <script> tags was incorrectly
flagged as JavaScript context XSS
- Credentials: Change from key-pattern matching to value-based analysis
- Add looksLikeActualCredential() to analyze if value looks like
a real credential (alphanumeric, no spaces, no non-ASCII)
- Skip display text (Japanese, sentences with spaces)
- Skip placeholder values (changeme, your_*_here, etc.)
- This fundamentally fixes false positives like:
'password_reset_mail_subject' => 'パスワードリセットのご案内'
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
@@ -41,6 +41,7 @@ class AuthenticationRule extends BaseRule
|
|||||||
'private_key', 'secret_key',
|
'private_key', 'secret_key',
|
||||||
];
|
];
|
||||||
|
|
||||||
|
|
||||||
/** @var array Patterns that indicate i18n/message keys (not credentials) */
|
/** @var array Patterns that indicate i18n/message keys (not credentials) */
|
||||||
private const I18N_KEY_PATTERNS = [
|
private const I18N_KEY_PATTERNS = [
|
||||||
'/^[a-z_]+\.[a-z_]+/', // dot notation like "auth.password"
|
'/^[a-z_]+\.[a-z_]+/', // dot notation like "auth.password"
|
||||||
@@ -505,6 +506,11 @@ class AuthenticationRule extends BaseRule
|
|||||||
if ($item->value instanceof Node\Scalar\String_) {
|
if ($item->value instanceof Node\Scalar\String_) {
|
||||||
$value = $item->value->value;
|
$value = $item->value->value;
|
||||||
|
|
||||||
|
// Skip empty values
|
||||||
|
if (empty($value)) {
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
|
||||||
// Skip Laravel validation rules
|
// Skip Laravel validation rules
|
||||||
if ($this->isLaravelValidationRule($value)) {
|
if ($this->isLaravelValidationRule($value)) {
|
||||||
continue;
|
continue;
|
||||||
@@ -515,7 +521,15 @@ class AuthenticationRule extends BaseRule
|
|||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
|
|
||||||
if (!empty($value) && !$this->isPlaceholder($value) && !$this->isDescriptiveMessage($value)) {
|
// Skip placeholder values
|
||||||
|
if ($this->isPlaceholder($value)) {
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
|
||||||
|
// Core check: does the VALUE look like an actual credential?
|
||||||
|
// This is the fundamental approach - analyze the value itself,
|
||||||
|
// not just the key name
|
||||||
|
if ($this->looksLikeActualCredential($value)) {
|
||||||
$vulnerabilities[] = $this->createVulnerability(
|
$vulnerabilities[] = $this->createVulnerability(
|
||||||
$this->msg('auth.name'),
|
$this->msg('auth.name'),
|
||||||
Vulnerability::SEVERITY_CRITICAL,
|
Vulnerability::SEVERITY_CRITICAL,
|
||||||
@@ -645,15 +659,30 @@ class AuthenticationRule extends BaseRule
|
|||||||
*/
|
*/
|
||||||
private function isPlaceholder(string $value): bool
|
private function isPlaceholder(string $value): bool
|
||||||
{
|
{
|
||||||
$placeholders = [
|
$lower = strtolower($value);
|
||||||
'xxx', 'password', 'secret', 'changeme', 'your_',
|
|
||||||
'your-', '<', '>', '{', '}', 'TODO', 'FIXME',
|
// Exact placeholder values (common development placeholders)
|
||||||
'env(', 'config(', 'getenv',
|
$exactPlaceholders = [
|
||||||
|
'xxx', 'xxxx', 'xxxxx', 'password', 'secret', 'changeme',
|
||||||
|
'change_me', 'changethis', 'your_password', 'your_secret',
|
||||||
|
'test', 'testing', 'example', 'sample', 'demo', 'dummy',
|
||||||
|
'null', 'none', 'empty', 'undefined',
|
||||||
];
|
];
|
||||||
|
|
||||||
$lower = strtolower($value);
|
if (in_array($lower, $exactPlaceholders)) {
|
||||||
foreach ($placeholders as $placeholder) {
|
return true;
|
||||||
if (str_contains($lower, strtolower($placeholder))) {
|
}
|
||||||
|
|
||||||
|
// Contains template syntax or environment references
|
||||||
|
$templatePatterns = [
|
||||||
|
'your_', 'your-', '<', '>', '{', '}', '${',
|
||||||
|
'TODO', 'FIXME', 'CHANGEME',
|
||||||
|
'env(', 'config(', 'getenv', 'process.env',
|
||||||
|
'_here', '_HERE', '-here', '-HERE',
|
||||||
|
];
|
||||||
|
|
||||||
|
foreach ($templatePatterns as $pattern) {
|
||||||
|
if (str_contains($value, $pattern) || str_contains($lower, strtolower($pattern))) {
|
||||||
return true;
|
return true;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -723,56 +752,82 @@ class AuthenticationRule extends BaseRule
|
|||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Check if value looks like a descriptive message rather than a credential
|
* Check if a value looks like an actual credential (not a label, message, or config value)
|
||||||
|
*
|
||||||
|
* This is the core detection logic: even if the KEY suggests a credential,
|
||||||
|
* we only flag it if the VALUE actually looks like a credential.
|
||||||
|
*
|
||||||
|
* Credential characteristics:
|
||||||
|
* - Relatively short string (typically < 100 chars)
|
||||||
|
* - No or few spaces (credentials are compact)
|
||||||
|
* - ASCII characters (or base64/hex encoded)
|
||||||
|
* - Not a human-readable sentence
|
||||||
|
*
|
||||||
|
* Non-credential (config/label) characteristics:
|
||||||
|
* - Contains spaces (human-readable text)
|
||||||
|
* - Contains non-ASCII (Japanese, etc.) - display text
|
||||||
|
* - Ends with punctuation (sentence)
|
||||||
|
* - Contains common message/label words
|
||||||
*/
|
*/
|
||||||
private function isDescriptiveMessage(string $value): bool
|
private function looksLikeActualCredential(string $value): bool
|
||||||
{
|
{
|
||||||
// Strings with multiple spaces are likely descriptions/messages
|
// Empty or very short values are not credentials
|
||||||
|
if (strlen($value) < 3) {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
|
||||||
|
// Very long strings are unlikely to be credentials (probably text content)
|
||||||
|
if (strlen($value) > 200) {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
|
||||||
|
// Contains non-ASCII characters (Japanese, Chinese, etc.) - likely display text
|
||||||
|
if (preg_match('/[^\x00-\x7F]/', $value)) {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
|
||||||
|
// Contains multiple spaces - likely a sentence or description
|
||||||
if (substr_count($value, ' ') >= 2) {
|
if (substr_count($value, ' ') >= 2) {
|
||||||
return true;
|
return false;
|
||||||
}
|
}
|
||||||
|
|
||||||
// Strings ending with punctuation are likely messages
|
// Ends with sentence punctuation - likely a message
|
||||||
if (preg_match('/[.!?。!?]$/', $value)) {
|
if (preg_match('/[.!?]$/', $value)) {
|
||||||
return true;
|
return false;
|
||||||
}
|
}
|
||||||
|
|
||||||
// Contains typical message indicators
|
// Contains common message/label indicators
|
||||||
$messageIndicators = [
|
$messageIndicators = [
|
||||||
'してください', // Japanese polite request
|
'please', 'should', 'must', 'warning', 'error', 'invalid',
|
||||||
'ください', // Japanese please
|
'expired', 'required', 'missing', 'detected', 'found', 'failed',
|
||||||
'です。', // Japanese sentence ending
|
'success', 'unable', 'cannot', 'not found', 'not allowed',
|
||||||
'ます。', // Japanese sentence ending
|
'the ', 'The ', 'a ', 'A ', 'an ', 'An ', 'is ', 'are ', 'was ',
|
||||||
'ません', // Japanese negative
|
'has ', 'have ', 'will ', 'would ', 'could ', 'can ',
|
||||||
'please',
|
'your ', 'Your ', 'this ', 'This ',
|
||||||
'should',
|
|
||||||
'must',
|
|
||||||
'warning',
|
|
||||||
'error',
|
|
||||||
'invalid',
|
|
||||||
'expired',
|
|
||||||
'required',
|
|
||||||
'missing',
|
|
||||||
'detected',
|
|
||||||
'found',
|
|
||||||
'failed',
|
|
||||||
'success',
|
|
||||||
'unable',
|
|
||||||
'cannot',
|
|
||||||
'not found',
|
|
||||||
'not allowed',
|
|
||||||
'use ',
|
|
||||||
'Use ',
|
|
||||||
];
|
];
|
||||||
|
|
||||||
$lower = strtolower($value);
|
$lower = strtolower($value);
|
||||||
foreach ($messageIndicators as $indicator) {
|
foreach ($messageIndicators as $indicator) {
|
||||||
if (str_contains($value, $indicator) || str_contains($lower, strtolower($indicator))) {
|
if (str_contains($value, $indicator) || str_contains($lower, strtolower($indicator))) {
|
||||||
return true;
|
return false;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
return false;
|
// If it looks like a typical credential format, it's suspicious
|
||||||
|
// Credentials are usually alphanumeric with possible special chars, no spaces
|
||||||
|
if (preg_match('/^[a-zA-Z0-9_\-+=\/\.@#$%^&*!]+$/', $value)) {
|
||||||
|
// Looks like a credential format
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
|
||||||
|
// Single word with a space (like "Reset Password") is likely a label
|
||||||
|
if (substr_count($value, ' ') === 1 && preg_match('/^[A-Z][a-z]+ [A-Z][a-z]+$/', $value)) {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
|
||||||
|
// Default: if it passed all the "not a credential" checks, consider it suspicious
|
||||||
|
// But only if it doesn't contain spaces (credentials typically don't have spaces)
|
||||||
|
return !str_contains($value, ' ');
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
|||||||
@@ -551,6 +551,21 @@ class InsecureConfigRule extends BaseRule
|
|||||||
if ($item->value instanceof Node\Scalar\String_) {
|
if ($item->value instanceof Node\Scalar\String_) {
|
||||||
$val = $item->value->value;
|
$val = $item->value->value;
|
||||||
|
|
||||||
|
// Skip empty values
|
||||||
|
if (empty($val)) {
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
|
||||||
|
// Skip language/translation files (values are field labels, not secrets)
|
||||||
|
if ($this->isLanguageFile($filePath)) {
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
|
||||||
|
// Skip if value looks like a translation (non-ASCII characters)
|
||||||
|
if ($this->isTranslationString($val)) {
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
|
||||||
// Skip i18n message keys
|
// Skip i18n message keys
|
||||||
if ($this->isI18nKey($originalKey)) {
|
if ($this->isI18nKey($originalKey)) {
|
||||||
continue;
|
continue;
|
||||||
@@ -566,8 +581,18 @@ class InsecureConfigRule extends BaseRule
|
|||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Skip placeholder values
|
||||||
|
if ($this->isPlaceholderValue($val)) {
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
|
||||||
|
// Core check: does the VALUE look like an actual credential?
|
||||||
|
if (!$this->looksLikeActualCredential($val)) {
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
|
||||||
// Check if it's not using env()
|
// Check if it's not using env()
|
||||||
if (!empty($val) && !str_starts_with($val, 'env(')) {
|
if (!str_starts_with($val, 'env(')) {
|
||||||
$vulnerabilities[] = $this->createVulnerability(
|
$vulnerabilities[] = $this->createVulnerability(
|
||||||
$this->msg('config.name'),
|
$this->msg('config.name'),
|
||||||
Vulnerability::SEVERITY_CRITICAL,
|
Vulnerability::SEVERITY_CRITICAL,
|
||||||
@@ -652,6 +677,120 @@ class InsecureConfigRule extends BaseRule
|
|||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Check if file is a language/translation file
|
||||||
|
*/
|
||||||
|
private function isLanguageFile(string $filePath): bool
|
||||||
|
{
|
||||||
|
// Common Laravel/PHP language file paths
|
||||||
|
$langPatterns = [
|
||||||
|
'#/lang/[a-z]{2}(_[A-Z]{2})?/#', // /lang/ja/, /lang/en/, /lang/zh_CN/
|
||||||
|
'#/locales?/[a-z]{2}(_[A-Z]{2})?/#', // /locale/ja/, /locales/en/
|
||||||
|
'#/resources/lang/#', // Laravel's resources/lang/
|
||||||
|
'#/translations?/#', // /translation/, /translations/
|
||||||
|
];
|
||||||
|
|
||||||
|
foreach ($langPatterns as $pattern) {
|
||||||
|
if (preg_match($pattern, $filePath)) {
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Check if value looks like a translation string (not a credential)
|
||||||
|
*/
|
||||||
|
private function isTranslationString(string $value): bool
|
||||||
|
{
|
||||||
|
if (empty($value)) {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
|
||||||
|
// Contains non-ASCII characters (Japanese, Chinese, Korean, etc.)
|
||||||
|
if (preg_match('/[^\x00-\x7F]/', $value)) {
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
|
||||||
|
// Common translation patterns: :attribute placeholder, readable text with spaces
|
||||||
|
if (preg_match('/:\w+/', $value)) { // :attribute, :max, etc.
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Check if value is a placeholder (not an actual credential)
|
||||||
|
*/
|
||||||
|
private function isPlaceholderValue(string $value): bool
|
||||||
|
{
|
||||||
|
$lower = strtolower($value);
|
||||||
|
|
||||||
|
// Exact placeholder values
|
||||||
|
$exactPlaceholders = [
|
||||||
|
'xxx', 'xxxx', 'xxxxx', 'password', 'secret', 'changeme',
|
||||||
|
'change_me', 'changethis', 'your_password', 'your_secret',
|
||||||
|
'test', 'testing', 'example', 'sample', 'demo', 'dummy',
|
||||||
|
'null', 'none', 'empty', 'undefined',
|
||||||
|
];
|
||||||
|
|
||||||
|
if (in_array($lower, $exactPlaceholders)) {
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
|
||||||
|
// Contains template syntax or environment references
|
||||||
|
$templatePatterns = [
|
||||||
|
'your_', 'your-', '<', '>', '{', '}', '${',
|
||||||
|
'TODO', 'FIXME', 'CHANGEME',
|
||||||
|
'env(', 'config(', 'getenv', 'process.env',
|
||||||
|
'_here', '_HERE', '-here', '-HERE',
|
||||||
|
];
|
||||||
|
|
||||||
|
foreach ($templatePatterns as $pattern) {
|
||||||
|
if (str_contains($value, $pattern) || str_contains($lower, strtolower($pattern))) {
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Check if a value looks like an actual credential
|
||||||
|
*/
|
||||||
|
private function looksLikeActualCredential(string $value): bool
|
||||||
|
{
|
||||||
|
// Very short values are not credentials
|
||||||
|
if (strlen($value) < 3) {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
|
||||||
|
// Very long strings are unlikely to be credentials
|
||||||
|
if (strlen($value) > 200) {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
|
||||||
|
// Contains non-ASCII characters - likely display text
|
||||||
|
if (preg_match('/[^\x00-\x7F]/', $value)) {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
|
||||||
|
// Contains multiple spaces - likely a sentence
|
||||||
|
if (substr_count($value, ' ') >= 2) {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
|
||||||
|
// Credentials are usually alphanumeric with possible special chars, no spaces
|
||||||
|
if (preg_match('/^[a-zA-Z0-9_\-+=\/\.@#$%^&*!]+$/', $value)) {
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
|
||||||
|
// Default: suspicious if no spaces
|
||||||
|
return !str_contains($value, ' ');
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Check config file return statements
|
* Check config file return statements
|
||||||
*/
|
*/
|
||||||
|
|||||||
@@ -211,42 +211,49 @@ class XssRule extends BaseRule
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// JavaScript context - {{ }} in <script> tags
|
// JavaScript context - Extract script blocks first, then check for Blade expressions
|
||||||
preg_match_all('/<script[^>]*>.*?\{\{(.+?)\}\}.*?<\/script>/s', $content, $jsMatches, PREG_OFFSET_CAPTURE);
|
// Use a pattern that doesn't cross script tag boundaries
|
||||||
|
preg_match_all('/<script[^>]*>((?:(?!<\/script>).)*)<\/script>/si', $content, $scriptBlocks, PREG_OFFSET_CAPTURE);
|
||||||
|
|
||||||
foreach ($jsMatches[1] as $match) {
|
foreach ($scriptBlocks[1] as $scriptBlock) {
|
||||||
$line = $this->getLineFromOffset($content, $match[1]);
|
$scriptContent = $scriptBlock[0];
|
||||||
$vulnerabilities[] = $this->createVulnerability(
|
$scriptOffset = $scriptBlock[1];
|
||||||
'XSS',
|
|
||||||
Vulnerability::SEVERITY_MEDIUM,
|
|
||||||
$this->msg('xss.blade_js_context'),
|
|
||||||
$filePath,
|
|
||||||
$line,
|
|
||||||
null,
|
|
||||||
$this->msg('xss.blade_js_context.rec'),
|
|
||||||
[],
|
|
||||||
'CWE-79',
|
|
||||||
'A7:2017-XSS'
|
|
||||||
);
|
|
||||||
}
|
|
||||||
|
|
||||||
// JavaScript context - {!! !!} in <script> tags (higher severity)
|
// Check for {{ }} in this script block
|
||||||
preg_match_all('/<script[^>]*>.*?\{!!(.+?)!!\}.*?<\/script>/s', $content, $jsRawMatches, PREG_OFFSET_CAPTURE);
|
preg_match_all('/\{\{(.+?)\}\}/s', $scriptContent, $jsMatches, PREG_OFFSET_CAPTURE);
|
||||||
|
foreach ($jsMatches[1] as $match) {
|
||||||
|
$line = $this->getLineFromOffset($content, $scriptOffset + $match[1]);
|
||||||
|
$vulnerabilities[] = $this->createVulnerability(
|
||||||
|
'XSS',
|
||||||
|
Vulnerability::SEVERITY_MEDIUM,
|
||||||
|
$this->msg('xss.blade_js_context'),
|
||||||
|
$filePath,
|
||||||
|
$line,
|
||||||
|
null,
|
||||||
|
$this->msg('xss.blade_js_context.rec'),
|
||||||
|
[],
|
||||||
|
'CWE-79',
|
||||||
|
'A7:2017-XSS'
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
foreach ($jsRawMatches[1] as $match) {
|
// Check for {!! !!} in this script block (higher severity)
|
||||||
$line = $this->getLineFromOffset($content, $match[1]);
|
preg_match_all('/\{!!(.+?)!!\}/s', $scriptContent, $jsRawMatches, PREG_OFFSET_CAPTURE);
|
||||||
$vulnerabilities[] = $this->createVulnerability(
|
foreach ($jsRawMatches[1] as $match) {
|
||||||
'XSS',
|
$line = $this->getLineFromOffset($content, $scriptOffset + $match[1]);
|
||||||
Vulnerability::SEVERITY_CRITICAL,
|
$vulnerabilities[] = $this->createVulnerability(
|
||||||
$this->msg('xss.blade_js_raw', ['expr' => trim($match[0])]),
|
'XSS',
|
||||||
$filePath,
|
Vulnerability::SEVERITY_CRITICAL,
|
||||||
$line,
|
$this->msg('xss.blade_js_raw', ['expr' => trim($match[0])]),
|
||||||
null,
|
$filePath,
|
||||||
$this->msg('xss.blade_js_raw.rec'),
|
$line,
|
||||||
[],
|
null,
|
||||||
'CWE-79',
|
$this->msg('xss.blade_js_raw.rec'),
|
||||||
'A7:2017-XSS'
|
[],
|
||||||
);
|
'CWE-79',
|
||||||
|
'A7:2017-XSS'
|
||||||
|
);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// URL context - href/src with user input (potential javascript: URLs)
|
// URL context - href/src with user input (potential javascript: URLs)
|
||||||
|
|||||||
Reference in New Issue
Block a user