Skip to content

Commit

Permalink
JS-431 Update S2068 (no-hardcoded-passwords): Adapt documentation t…
Browse files Browse the repository at this point in the history
…o focus only on passwords (#4931)
  • Loading branch information
yassin-kammoun-sonarsource authored Nov 27, 2024
1 parent c0dee46 commit 2747614
Show file tree
Hide file tree
Showing 15 changed files with 54 additions and 56 deletions.
3 changes: 2 additions & 1 deletion packages/jsts/src/rules/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -230,8 +230,9 @@ If you are a [SonarQube](https://www.sonarqube.org) or [SonarCloud](https://sona
| [no-global-this](https://sonarsource.github.io/rspec/#/rspec/S2990/javascript) | The global "this" object should not be used || | 💡 | | |
| [no-globals-shadowing](https://sonarsource.github.io/rspec/#/rspec/S2137/javascript) | Special identifiers should not be bound or assigned || | | | |
| [no-gratuitous-expressions](https://sonarsource.github.io/rspec/#/rspec/S2589/javascript) | Boolean expressions should not be gratuitous || | | | |
| [no-hardcoded-credentials](https://sonarsource.github.io/rspec/#/rspec/S2068/javascript) | Hard-coded credentials are security-sensitive || | | | |
| [no-hardcoded-ip](https://sonarsource.github.io/rspec/#/rspec/S1313/javascript) | Using hardcoded IP addresses is security-sensitive || | | | |
| [no-hardcoded-passwords](https://sonarsource.github.io/rspec/#/rspec/S2068/javascript) | Hard-coded passwords are security-sensitive || | | | |
| [no-hardcoded-secrets](https://sonarsource.github.io/rspec/#/rspec/S6418/javascript) | Hard-coded secrets are security-sensitive || | | | |
| [no-hook-setter-in-body](https://sonarsource.github.io/rspec/#/rspec/S6442/javascript) | React's useState hook should not be used directly in the render function or body of a component || | | | |
| [no-identical-conditions](https://sonarsource.github.io/rspec/#/rspec/S1862/javascript) | "if/else if" chains and "switch" cases should not have the same condition || | | | |
| [no-identical-expressions](https://sonarsource.github.io/rspec/#/rspec/S1764/javascript) | Identical expressions should not be used on both sides of a binary operator || | | | |
Expand Down
4 changes: 2 additions & 2 deletions packages/jsts/src/rules/S2068/meta.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
export const meta = {
type: 'problem',
docs: {
description: 'Hard-coded credentials are security-sensitive',
description: 'Hard-coded passwords are security-sensitive',
recommended: true,
url: 'https://sonarsource.github.io/rspec/#/rspec/S2068/javascript',
requiresTypeChecking: false,
Expand All @@ -36,7 +36,7 @@ export const schema = {
{
type: 'object',
properties: {
credentialWords: {
passwordWords: {
type: 'array',
items: {
type: 'string',
Expand Down
8 changes: 4 additions & 4 deletions packages/jsts/src/rules/S2068/rule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import { meta, schema } from './meta.js';
const DEFAULT_NAMES = ['password', 'pwd', 'passwd'];

const messages = {
reviewCredential: 'Review this potentially hardcoded credential.',
reviewPassword: 'Review this potentially hard-coded password.',
};

export const rule: Rule.RuleModule = {
Expand All @@ -39,7 +39,7 @@ export const rule: Rule.RuleModule = {
}

const variableNames =
(context.options as FromSchema<typeof schema>)[0]?.credentialWords ?? DEFAULT_NAMES;
(context.options as FromSchema<typeof schema>)[0]?.passwordWords ?? DEFAULT_NAMES;
const literalRegExp = variableNames.map(name => new RegExp(`${name}=.+`));
return {
VariableDeclarator: (node: estree.Node) => {
Expand Down Expand Up @@ -75,7 +75,7 @@ function checkAssignment(
patterns.some(pattern => context.sourceCode.getText(variable).includes(pattern))
) {
context.report({
messageId: 'reviewCredential',
messageId: 'reviewPassword',
node: initializer,
});
}
Expand All @@ -84,7 +84,7 @@ function checkAssignment(
function checkLiteral(context: Rule.RuleContext, patterns: RegExp[], literal: estree.Literal) {
if (isStringLiteral(literal) && patterns.some(pattern => pattern.test(literal.value as string))) {
context.report({
messageId: 'reviewCredential',
messageId: 'reviewPassword',
node: literal,
});
}
Expand Down
12 changes: 6 additions & 6 deletions packages/jsts/src/rules/S2068/unit.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,9 @@ const ruleTester = new NodeRuleTester({
parserOptions: { ecmaVersion: 2018, sourceType: 'module' },
});

const options = [{ credentialWords: ['password', 'pwd', 'passwd'] }];
const options = [{ passwordWords: ['password', 'pwd', 'passwd'] }];

ruleTester.run('Hardcoded credentials should be avoided', rule, {
ruleTester.run('Hard-coded passwords should be avoided', rule, {
valid: [
{
code: `let password = ""`,
Expand All @@ -44,7 +44,7 @@ ruleTester.run('Hardcoded credentials should be avoided', rule, {
options,
errors: [
{
message: 'Review this potentially hardcoded credential.',
message: 'Review this potentially hard-coded password.',
line: 1,
endLine: 1,
column: 16,
Expand All @@ -66,7 +66,7 @@ ruleTester.run('Hardcoded credentials should be avoided', rule, {
errors: 1,
},
{
code: `let credentials = { user: "foo", passwd: "bar" };`,
code: `let passwords = { user: "foo", passwd: "bar" };`,
options,
errors: 1,
},
Expand All @@ -77,12 +77,12 @@ ruleTester.run('Hardcoded credentials should be avoided', rule, {
},
{
code: `let secret = "foo"`,
options: [{ credentialWords: ['secret'] }],
options: [{ passwordWords: ['secret'] }],
errors: 1,
},
{
code: `let url = "https://example.com?token=hl2OAIXXZ60";`,
options: [{ credentialWords: ['token'] }],
options: [{ passwordWords: ['token'] }],
errors: 1,
},
{
Expand Down
2 changes: 1 addition & 1 deletion packages/jsts/src/rules/S6418/rule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ function buildSecretWordRegexps(secretWords: string) {
return secretWords.split(',').map(word => new RegExp(`(${word})`, 'i'));
} catch (e) {
console.error(
`Invalid characters provided to rule S6418 'hardcoded-secrets' parameter "secretWords": "${secretWords}" falling back to default: "${DEFAULT_SECRET_WORDS}". Error: ${e}`,
`Invalid characters provided to rule S6418 'no-hardcoded-secrets' parameter "secretWords": "${secretWords}" falling back to default: "${DEFAULT_SECRET_WORDS}". Error: ${e}`,
);
return buildSecretWordRegexps(DEFAULT_SECRET_WORDS);
}
Expand Down
2 changes: 1 addition & 1 deletion packages/jsts/src/rules/S6418/unit.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import { rule } from './rule.js';

const ruleTester = new JavaScriptRuleTester();

ruleTester.run('Rule S6418 - hardcoded-secrets', rule, {
ruleTester.run('Rule S6418 - no-hardcoded-secrets', rule, {
valid: [],
invalid: [
// we're verifying that given a broken RegExp, the rule still works.
Expand Down
4 changes: 2 additions & 2 deletions packages/jsts/src/rules/original.ts
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,6 @@ export { rule as S100 } from './S100/index.js'; // function-name
export { rule as S3800 } from './S3800/index.js'; // function-return-type
export { rule as S1527 } from './S1527/index.js'; // future-reserved-words
export { rule as S3531 } from './S3531/index.js'; // generator-without-yield
export { rule as S6418 } from './S6418/index.js'; // hard-coded-credentials
export { rule as S4790 } from './S4790/index.js'; // hashing
export { rule as S5691 } from './S5691/index.js'; // hidden-files
export { rule as S6754 } from './S6754/index.js'; // hook-use-state
Expand Down Expand Up @@ -146,8 +145,9 @@ export { rule as S1530 } from './S1530/index.js'; // no-function-declaration-in-
export { rule as S2990 } from './S2990/index.js'; // no-global-this
export { rule as S2137 } from './S2137/index.js'; // no-globals-shadowing
export { rule as S2589 } from './S2589/index.js'; // no-gratuitous-expressions
export { rule as S2068 } from './S2068/index.js'; // no-hardcoded-credentials
export { rule as S1313 } from './S1313/index.js'; // no-hardcoded-ip
export { rule as S2068 } from './S2068/index.js'; // no-hardcoded-passwords
export { rule as S6418 } from './S6418/index.js'; // no-hardcoded-secrets
export { rule as S6442 } from './S6442/index.js'; // no-hook-setter-in-body
export { rule as S1862 } from './S1862/index.js'; // no-identical-conditions
export { rule as S1764 } from './S1764/index.js'; // no-identical-expressions
Expand Down
4 changes: 2 additions & 2 deletions packages/jsts/src/rules/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,6 @@ export const rules: Record<string, Rule.RuleModule> = {
'function-return-type': originalRules.S3800,
'future-reserved-words': originalRules.S1527,
'generator-without-yield': originalRules.S3531,
'hardcoded-credentials': originalRules.S6418,
hashing: originalRules.S4790,
'hidden-files': originalRules.S5691,
'hook-use-state': originalRules.S6754,
Expand Down Expand Up @@ -156,8 +155,9 @@ export const rules: Record<string, Rule.RuleModule> = {
'no-global-this': originalRules.S2990,
'no-globals-shadowing': originalRules.S2137,
'no-gratuitous-expressions': originalRules.S2589,
'no-hardcoded-credentials': originalRules.S2068,
'no-hardcoded-ip': originalRules.S1313,
'no-hardcoded-passwords': originalRules.S2068,
'no-hardcoded-secrets': originalRules.S6418,
'no-hook-setter-in-body': originalRules.S6442,
'no-identical-conditions': originalRules.S1862,
'no-identical-expressions': originalRules.S1764,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -180,8 +180,6 @@ public static List<Class<? extends JavaScriptCheck>> getAllChecks() {
GlobalThisCheck.class,
GlobalsShadowingCheck.class,
GratuitousConditionCheck.class,
HardcodedPasswordCheck.class,
HardcodedSecretsCheck.class,
HashingCheck.class,
HeadingHasContentCheck.class,
HiddenFilesCheck.class,
Expand Down Expand Up @@ -277,6 +275,8 @@ public static List<Class<? extends JavaScriptCheck>> getAllChecks() {
NoFindDomNodeCheck.class,
NoForInArrayCheck.class,
NoHardcodedIpCheck.class,
NoHardcodedPasswordsCheck.class,
NoHardcodedSecretsCheck.class,
NoHookSetterInBodyCheck.class,
NoIgnoredExceptionsCheck.class,
NoImportAssignCheck.class,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,28 +27,28 @@
@JavaScriptRule
@TypeScriptRule
@Rule(key = "S2068")
public class HardcodedPasswordCheck extends Check {
public class NoHardcodedPasswordsCheck extends Check {

private static final String DEFAULT = "password, pwd, passwd";

@RuleProperty(
key = "credentialWords",
description = "Comma separated list of words identifying potential credentials.",
key = "passwordWords",
description = "Comma separated list of words identifying potential passwords.",
defaultValue = "" + DEFAULT
)
public String credentialWords = DEFAULT;
public String passwordWords = DEFAULT;

@Override
public List<Object> configurations() {
return Collections.singletonList(
new Config(credentialWords.split("\\s*,\\s*"))
new Config(passwordWords.split("\\s*,\\s*"))
);
}

private static class Config {
String[] credentialWords;
Config(String[] credentialWords) {
this.credentialWords = credentialWords;
String[] passwordWords;
Config(String[] passwordWords) {
this.passwordWords = passwordWords;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
@TypeScriptRule
@JavaScriptRule
@Rule(key = "S6418")
public class HardcodedSecretsCheck extends Check {
public class NoHardcodedSecretsCheck extends Check {

private static final String DEFAULT_SECRET_WORDS = "api[_.-]?key,auth,credential,secret,token";
private static final String DEFAULT_RANDOMNESS_SENSIBILITY = "5.0";
Expand Down
Original file line number Diff line number Diff line change
@@ -1,34 +1,32 @@
<p>Because it is easy to extract strings from an application source code or binary, credentials should not be hard-coded. This is particularly true
for applications that are distributed or that are open-source.</p>
<p>Because it is easy to extract strings from an application source code or binary, passwords should not be hard-coded. This is particularly true for
applications that are distributed or that are open-source.</p>
<p>In the past, it has led to the following vulnerabilities:</p>
<ul>
<li> <a href="http://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2019-13466">CVE-2019-13466</a> </li>
<li> <a href="http://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2018-15389">CVE-2018-15389</a> </li>
</ul>
<p>Credentials should be stored outside of the code in a configuration file, a database, or a management service for secrets.</p>
<p>This rule flags instances of hard-coded credentials used in database and LDAP connections. It looks for hard-coded credentials in connection
strings, and for variable names that match any of the patterns from the provided list.</p>
<p>It’s recommended to customize the configuration of this rule with additional credential words such as "oauthToken", "secret", …​</p>
<p>Passwords should be stored outside of the code in a configuration file, a database, or a management service for passwords.</p>
<p>This rule flags instances of hard-coded passwords used in database and LDAP connections. It looks for hard-coded passwords in connection strings,
and for variable names that match any of the patterns from the provided list.</p>
<h2>Ask Yourself Whether</h2>
<ul>
<li> Credentials allow access to a sensitive component like a database, a file storage, an API or a service. </li>
<li> Credentials are used in production environments. </li>
<li> Application re-distribution is required before updating the credentials. </li>
<li> Passwords allow access to a sensitive component like a database, a file storage, an API or a service. </li>
<li> Passwords are used in production environments. </li>
<li> Application re-distribution is required before updating the passwords. </li>
</ul>
<p>There is a risk if you answered yes to any of those questions.</p>
<h2>Recommended Secure Coding Practices</h2>
<ul>
<li> Store the credentials in a configuration file that is not pushed to the code repository. </li>
<li> Store the credentials in a database. </li>
<li> Use your cloud provider’s service for managing secrets. </li>
<li> Store the passwords in a configuration file that is not pushed to the code repository. </li>
<li> Store the passwords in a database. </li>
<li> Use your cloud provider’s service for managing passwords. </li>
<li> If a password has been disclosed through the source code: change it. </li>
</ul>
<h2>Sensitive Code Example</h2>
<pre>
var mysql = require('mysql');
const mysql = require('mysql');

var connection = mysql.createConnection(
{
const connection = mysql.createConnection({
host:'localhost',
user: "admin",
database: "project",
Expand All @@ -40,9 +38,9 @@ <h2>Sensitive Code Example</h2>
</pre>
<h2>Compliant Solution</h2>
<pre>
var mysql = require('mysql');
const mysql = require('mysql');

var connection = mysql.createConnection({
const connection = mysql.createConnection({
host: process.env.MYSQL_URL,
user: process.env.MYSQL_USERNAME,
password: process.env.MYSQL_PASSWORD,
Expand All @@ -56,7 +54,6 @@ <h2>See</h2>
Authentication Failures</a> </li>
<li> OWASP - <a href="https://owasp.org/www-project-top-ten/2017/A2_2017-Broken_Authentication">Top 10 2017 Category A2 - Broken Authentication</a>
</li>
<li> CWE - <a href="https://cwe.mitre.org/data/definitions/798">CWE-798 - Use of Hard-coded Credentials</a> </li>
<li> CWE - <a href="https://cwe.mitre.org/data/definitions/259">CWE-259 - Use of Hard-coded Password</a> </li>
<li> Derived from FindSecBugs rule <a href="https://h3xstream.github.io/find-sec-bugs/bugs.htm#HARD_CODE_PASSWORD">Hard Coded Password</a> </li>
</ul>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"title": "Hard-coded credentials are security-sensitive",
"title": "Hard-coded passwords are security-sensitive",
"type": "SECURITY_HOTSPOT",
"code": {
"impacts": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,17 +21,17 @@
import com.google.gson.Gson;
import org.junit.jupiter.api.Test;

class HardcodedPasswordCheckTest {
class NoHardcodedPasswordsCheckTest {

@Test
void configurations() {
HardcodedPasswordCheck check = new HardcodedPasswordCheck();
NoHardcodedPasswordsCheck check = new NoHardcodedPasswordsCheck();
// default configuration
String defaultConfigAsString = new Gson().toJson(check.configurations());
assertThat(defaultConfigAsString).isEqualTo("[{\"credentialWords\":[\"password\",\"pwd\",\"passwd\"]}]");
assertThat(defaultConfigAsString).isEqualTo("[{\"passwordWords\":[\"password\",\"pwd\",\"passwd\"]}]");

check.credentialWords = "foo, bar";
check.passwordWords = "foo, bar";
String customConfigAsString = new Gson().toJson(check.configurations());
assertThat(customConfigAsString).isEqualTo("[{\"credentialWords\":[\"foo\",\"bar\"]}]");
assertThat(customConfigAsString).isEqualTo("[{\"passwordWords\":[\"foo\",\"bar\"]}]");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,11 @@
import com.google.gson.Gson;
import org.junit.jupiter.api.Test;

class HardcodedSecretsCheckTest {
class NoHardcodedSecretsCheckTest {

@Test
void configurations() {
HardcodedSecretsCheck check = new HardcodedSecretsCheck();
NoHardcodedSecretsCheck check = new NoHardcodedSecretsCheck();
// default configuration
String defaultConfigAsString = new Gson().toJson(check.configurations());
assertThat(defaultConfigAsString).isEqualTo(
Expand Down

0 comments on commit 2747614

Please sign in to comment.