Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JS-431 Update S2068 (no-hardcoded-passwords): Adapt documentation to focus only on passwords #4931

Merged
merged 1 commit into from
Nov 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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