skills/semgrep__skills
1
0
Fork 0
mirror of https://github.com/semgrep/skills.git synced 2026-09-20 14:21:26 +08:00
Code Issues Packages Projects Releases Wiki Activity

reduce size of files

Browse Source
This commit is contained in:
Drew Dennison
2026-01-15 12:54:30 -08:00
parent c81e69ac77
commit 1804f863e2
27 changed files with 4180 additions and 51045 deletions
Download Patch File Download Diff File Expand all files Collapse all files
packages/code-security-build/test-cases.json
+706 -7098
View File
File diff suppressed because it is too large Load Diff
skills/code-security/AGENTS.md
+1738 -22039
View File
File diff suppressed because it is too large Load Diff
skills/code-security/rules/authentication-jwt.md
+28 -297
View File
@@ -5,17 +5,19 @@ impact: HIGH
## Secure JWT Authentication
JSON Web Tokens (JWT) are widely used for authentication and authorization. However, improper implementation can lead to serious security vulnerabilities including authentication bypass and token forgery. The most critical JWT vulnerability is decoding tokens without verifying their signatures, which allows attackers to forge tokens with arbitrary claims, impersonate any user, or escalate privileges. Other vulnerabilities include accepting the "none" algorithm, using weak secrets, and accepting unsigned tokens. Related CWEs: CWE-287 (Improper Authentication), CWE-345 (Insufficient Verification of Data Authenticity), CWE-347 (Improper Verification of Cryptographic Signature).
JSON Web Tokens (JWT) are widely used for authentication and authorization. However, improper implementation can lead to serious security vulnerabilities including authentication bypass and token forgery. The most critical JWT vulnerability is decoding tokens without verifying their signatures, which allows attackers to forge tokens with arbitrary claims, impersonate any user, or escalate privileges.
Related CWEs: CWE-287 (Improper Authentication), CWE-345 (Insufficient Verification of Data Authenticity), CWE-347 (Improper Verification of Cryptographic Signature).
**Incorrect (JavaScript jsonwebtoken - decode without verify):**
```javascript
const jwt = require('jsonwebtoken');
function notOk(token) {
// ruleid: jwt-decode-without-verify
if (jwt.decode(token, true).param === true) {
console.log('token is valid');
function getUserData(token) {
const decoded = jwt.decode(token, true);
if (decoded.isAdmin) {
return getAdminData();
}
}
```
@@ -25,176 +27,45 @@ function notOk(token) {
```javascript
const jwt = require('jsonwebtoken');
function ok(token, key) {
// ok: jwt-decode-without-verify
jwt.verify(token, key);
if (jwt.decode(token, true).param === true) {
console.log('token is valid');
function getUserData(token, secretKey) {
jwt.verify(token, secretKey);
const decoded = jwt.decode(token, true);
if (decoded.isAdmin) {
return getAdminData();
}
}
```
**Incorrect (JavaScript jwt-simple - verification disabled):**
```javascript
const jwt = require('jwt-simple');
const secretKey = process.env.JWT_SECRET;
// Route that requires authentication
app.get('/protectedRoute1', (req, res) => {
const token = req.headers.authorization;
if (!token) {
return res.status(401).json({ error: 'Unauthorized. Token missing.' });
}
try {
// ruleid: jwt-simple-noverify
const decoded = jwt.decode(token, secretKey, 'HS256');
res.json({ message: `Hello ${decoded.username}` });
} catch (error) {
res.status(401).json({ error: 'Unauthorized. Invalid token.' });
}
});
// Also incorrect - passing true disables verification
app.get('/protectedRoute2', (req, res) => {
const token = req.headers.authorization;
try {
// ruleid: jwt-simple-noverify
const decoded = jwt.decode(token, secretKey, true);
res.json({ message: `Hello ${decoded.username}` });
} catch (error) {
res.status(401).json({ error: 'Unauthorized. Invalid token.' });
}
});
// Also incorrect - string 'false' is truthy
app.get('/protectedRoute3', (req, res) => {
const token = req.headers.authorization;
try {
// ruleid: jwt-simple-noverify
const decoded = jwt.decode(token, secretKey, 'false');
res.json({ message: `Hello ${decoded.username}` });
} catch (error) {
res.status(401).json({ error: 'Unauthorized. Invalid token.' });
}
});
```
**Correct (JavaScript jwt-simple - verification enabled):**
```javascript
const jwt = require('jwt-simple');
const secretKey = process.env.JWT_SECRET;
// Route that requires authentication - default verification
app.get('/protectedRoute4', (req, res) => {
const token = req.headers.authorization;
if (!token) {
return res.status(401).json({ error: 'Unauthorized. Token missing.' });
}
try {
// ok: jwt-simple-noverify
const decoded = jwt.decode(token, secretKey);
res.json({ message: `Hello ${decoded.username}` });
} catch (error) {
res.status(401).json({ error: 'Unauthorized. Invalid token.' });
}
});
// Explicitly enable verification with false
app.get('/protectedRoute5', (req, res) => {
const token = req.headers.authorization;
try {
// ok: jwt-simple-noverify
const decoded = jwt.decode(token, secretKey, false);
res.json({ message: `Hello ${decoded.username}` });
} catch (error) {
res.status(401).json({ error: 'Unauthorized. Invalid token.' });
}
});
```
**Incorrect (Python PyJWT - verify_signature disabled):**
```python
import jwt
from jwt.exceptions import DecodeError, MissingRequiredClaimError, InvalidKeyError
def tests(token):
# ruleid:unverified-jwt-decode
jwt.decode(encoded, key, options={"verify_signature": False})
# ruleid:unverified-jwt-decode
opts = {"verify_signature": False}
jwt.decode(encoded, key, options=opts)
a_false_boolean = False
# ruleid:unverified-jwt-decode
opts2 = {"verify_signature": a_false_boolean}
jwt.decode(encoded, key, options=opts2)
def get_user_claims(token, key):
decoded = jwt.decode(token, key, options={"verify_signature": False})
return decoded
```
**Correct (Python PyJWT - verify_signature enabled):**
```python
import jwt
from jwt.exceptions import DecodeError, MissingRequiredClaimError, InvalidKeyError
def tests(token):
# ok:unverified-jwt-decode
jwt.decode(encoded, key, options={"verify_signature": True})
opts = {"verify_signature": True}
# ok:unverified-jwt-decode
jwt.decode(encoded, key, options=opts)
a_false_boolean = True
opts2 = {"verify_signature": a_false_boolean}
# ok:unverified-jwt-decode
jwt.decode(encoded, key, options=opts2)
# ok:unverified-jwt-decode - default is to verify
jwt.decode(encoded, key)
def get_user_claims(token, key):
decoded = jwt.decode(token, key, algorithms=["HS256"])
return decoded
```
**Incorrect (Java auth0 java-jwt - decode without verify):**
```java
package jwt_test.jwt_test_1;
import com.auth0.jwt.JWT;
import com.auth0.jwt.algorithms.Algorithm;
import com.auth0.jwt.exceptions.JWTCreationException;
import com.auth0.jwt.interfaces.DecodedJWT;
import com.auth0.jwt.interfaces.JWTVerifier;
abstract class App2
{
private void bad( String[] args )
{
System.out.println( "Hello World!" );
try {
Algorithm algorithm = Algorithm.none();
String token = JWT.create()
.withIssuer("auth0")
.sign(algorithm);
// ruleid: java-jwt-decode-without-verify
DecodedJWT jwt = JWT.decode(token);
} catch (JWTCreationException exception){
//Invalid Signing configuration / Couldn't convert Claims.
}
public class TokenHandler {
public DecodedJWT getUserClaims(String token) {
DecodedJWT jwt = JWT.decode(token);
return jwt;
}
}
```
@@ -202,165 +73,25 @@ abstract class App2
**Correct (Java auth0 java-jwt - verify before use):**
```java
package jwt_test.jwt_test_1;
import com.auth0.jwt.JWT;
import com.auth0.jwt.algorithms.Algorithm;
import com.auth0.jwt.exceptions.JWTCreationException;
import com.auth0.jwt.interfaces.DecodedJWT;
import com.auth0.jwt.interfaces.JWTVerifier;
public class App
{
private void verifyToken(String token, String secret) {
public class TokenHandler {
public DecodedJWT getUserClaims(String token, String secret) {
Algorithm algorithm = Algorithm.HMAC256(secret);
JWTVerifier verifier = JWT.require(algorithm)
.withIssuer("auth0")
.build(); //Reusable verifier instance
DecodedJWT jwt2 = verifier.verify(token);
}
public void ok( String[] args )
{
System.out.println( "Hello World!" );
try {
Algorithm algorithm = Algorithm.HMAC256(args[0]);
String token = JWT.create()
.withIssuer("auth0")
.sign(algorithm);
// Decode only after verification in verifyToken()
DecodedJWT jwt = JWT.decode(token);
} catch (JWTCreationException exception){
//Invalid Signing configuration / Couldn't convert Claims.
}
.withIssuer("auth0")
.build();
DecodedJWT jwt = verifier.verify(token);
return jwt;
}
}
```
**Incorrect (Go jwt-go - ParseUnverified):**
```go
package main
import (
"fmt"
"github.com/dgrijalva/jwt-go"
)
func bad1(tokenString string) {
// ruleid: jwt-go-parse-unverified
token, _, err := new(jwt.Parser).ParseUnverified(tokenString, jwt.MapClaims{})
if err != nil {
fmt.Println(err)
return
}
if claims, ok := token.Claims.(jwt.MapClaims); ok {
fmt.Println(claims["foo"], claims["exp"])
} else {
fmt.Println(err)
}
}
```
**Correct (Go jwt-go - ParseWithClaims):**
```go
package main
import (
"fmt"
"github.com/dgrijalva/jwt-go"
)
func ok1(tokenString string, keyFunc Keyfunc) {
// ok: jwt-go-parse-unverified
token, err := new(jwt.Parser).ParseWithClaims(tokenString, jwt.MapClaims{}, keyFunc)
if err != nil {
fmt.Println(err)
return
}
if claims, ok := token.Claims.(jwt.MapClaims); ok {
fmt.Println(claims["foo"], claims["exp"])
} else {
fmt.Println(err)
}
}
```
**Incorrect (Ruby ruby-jwt - verification disabled):**
```ruby
require 'jwt'
def bad1(hmac_secret)
# ruleid: ruby-jwt-decode-without-verify
decoded_token = JWT.decode token, hmac_secret, false, { algorithm: 'HS256' }
puts decoded_token
end
```
**Correct (Ruby ruby-jwt - verification enabled):**
```ruby
require 'jwt'
def ok1(hmac_secret)
# ok: ruby-jwt-decode-without-verify
token = JWT.encode payload, hmac_secret, 'HS256'
puts token
decoded_token = JWT.decode token, hmac_secret, true, { algorithm: 'HS256' }
puts decoded_token
end
```
**Incorrect (C# TokenValidationParameters - unsigned tokens accepted):**
```csharp
services.AddAuthentication(JwtBearerDefaults.AuthenticationScheme).AddJwtBearer(options =>
{
options.TokenValidationParameters = new TokenValidationParameters
{
// ruleid: unsigned-security-token
RequireSignedTokens = false,
ValidateIssuer = false,
ValidateAudience = false
};
});
```
**Correct (C# TokenValidationParameters - signed tokens required):**
```csharp
services.AddAuthentication(JwtBearerDefaults.AuthenticationScheme).AddJwtBearer(options =>
{
options.TokenValidationParameters = new TokenValidationParameters
{
// ok: unsigned-security-token
RequireSignedTokens = true,
ValidateIssuer = false,
ValidateAudience = false
};
});
```
References:
- https://owasp.org/Top10/A08_2021-Software_and_Data_Integrity_Failures
- https://owasp.org/Top10/A01_2021-Broken_Access_Control/
- https://owasp.org/Top10/A02_2021-Cryptographic_Failures/
- https://semgrep.dev/blog/2020/hardcoded-secrets-unverified-tokens-and-other-common-jwt-mistakes/
- https://cwe.mitre.org/data/definitions/287
- https://cwe.mitre.org/data/definitions/345
- https://cwe.mitre.org/data/definitions/347
- https://www.npmjs.com/package/jwt-simple
- https://github.com/we45/Vulnerable-Flask-App/blob/752ee16087c0bfb79073f68802d907569a1f0df7/app/app.py#L96
skills/code-security/rules/best-practice.md
+85 -761
View File
@@ -7,834 +7,158 @@ impact: LOW
This document outlines coding best practices across multiple languages. Following these patterns helps improve code quality, maintainability, and prevents common mistakes.
**Incorrect (Python - file not closed):**
### File Handling - Always Close Files
**Incorrect (Python):**
```python
def func1():
# ruleid:open-never-closed
fd = open('foo')
x = 123
```
**Correct (Python - file properly closed):**
**Correct (Python - using context manager):**
```python
def func2():
# ok:open-never-closed
fd = open('bar')
fd.close()
def func3():
# ok:open-never-closed
fd = open('baz')
try:
pass
finally:
fd.close()
with open('bar', encoding='utf-8') as fd:
data = fd.read()
```
**Incorrect (Python - unspecified encoding):**
### Specify File Encoding
`open()` uses device locale encodings by default, corrupting files with special characters. Specify the encoding to ensure cross-platform support when opening files in text mode.
`open()` uses device locale encodings by default. Always specify encoding in text mode.
**Incorrect:**
```python
def func1():
# ruleid:unspecified-open-encoding
fd = open('foo')
fd.close()
def func2():
# ruleid:unspecified-open-encoding
fd = open('foo', mode="w")
fd.close()
fd = open('foo', mode="w")
```
**Correct (Python - encoding specified):**
**Correct:**
```python
def func7():
# ok:unspecified-open-encoding
fd = open('foo', encoding='utf-8')
fd.close()
def func8():
# ok:unspecified-open-encoding
fd = open('foo', encoding="utf-8", mode="w")
fd.close()
fd = open('foo', encoding='utf-8', mode="w")
```
**References:**
- https://www.python.org/dev/peps/pep-0597/
- https://docs.python.org/3/library/functions.html#open
### Network Requests Need Timeouts
**Incorrect (Python - missing __hash__ with __eq__):**
Requests without a timeout will hang indefinitely if no response is received.
Class that has defined `__eq__` should also define `__hash__` for proper behavior in sets and as dictionary keys.
```python
# ruleid:missing-hash-with-eq
class A:
def __eq__(self, someother):
pass
```
**Correct (Python - __hash__ defined with __eq__):**
```python
# ok:missing-hash-with-eq
class A2:
def __eq__(self, someother):
pass
def __hash__(self):
pass
```
**Incorrect (Python - empty pass body):**
`pass` as the body of a function or loop is often a mistake or unfinished code.
```python
# ruleid:pass-body-range
for i in range(100):
pass
# ruleid:pass-body-fn
def foo():
pass
```
**Correct (Python - appropriate use of pass):**
```python
def __init__(self):
# ok:pass-body-fn
pass
class foo:
def somemethod():
# ok:pass-body-fn
pass
```
**Incorrect (Python - requests without timeout):**
`requests` calls without a timeout will hang the program if a response is never received. Always set a timeout for all requests.
**Incorrect (Python):**
```python
import requests
url = "www.github.com"
# ruleid: use-timeout
r = requests.get(url)
# ruleid: use-timeout
r = requests.post(url)
```
**Correct (Python - requests with timeout):**
**Correct (Python):**
```python
# ok: use-timeout
r = requests.get(url, timeout=50)
def from_import_test1(url):
from requests import get, post
# ok: use-timeout
r = get(url, timeout=3)
r = requests.get(url, timeout=30)
```
**References:**
- https://docs.python-requests.org/en/latest/user/advanced/?highlight=timeout#timeouts
### Remove Debug Statements
**Incorrect (Django - HttpResponse with json.dumps):**
Debug statements like `alert()`, `confirm()`, `prompt()`, and `debugger` should not be in production code.
Use Django's `JsonResponse` helper instead of manually serializing JSON.
**Incorrect (JavaScript):**
```python
from django.http import HttpResponse
import json
def foo():
# ruleid:use-json-response
dump = json.dumps({})
return HttpResponse(dump, content_type='application/json')
```javascript
var name = prompt('what is your name');
alert('your name is ' + name);
debugger;
```
**Incorrect (Flask - json.dumps instead of jsonify):**
### Load Modules at Top Level
`flask.jsonify()` is a Flask helper method which handles the correct settings for returning JSON from Flask routes.
Lazy loading inside functions complicates bundling and blocks requests synchronously in Node.js.
```python
import flask
import json
app = flask.Flask(__name__)
@app.route("/user")
def user():
user_dict = get_user(request.args.get("id"))
# ruleid:use-jsonify
return json.dumps(user_dict)
```
**References:**
- https://flask.palletsprojects.com/en/2.2.x/api/#flask.json.jsonify
**Incorrect (JavaScript - lazy loading modules inside functions):**
Lazy loading can complicate code bundling. `require` calls are run synchronously by Node.js and may block other requests when called from within a function.
**Incorrect (JavaScript):**
```javascript
function smth() {
// ruleid: lazy-load-module
const mod = require('module-name')
return mod();
}
```
**Correct (JavaScript - modules loaded at top level):**
**Correct (JavaScript):**
```javascript
// ok: lazy-load-module
const fs = require('fs')
```
**References:**
- https://nodesecroadmap.fyi/chapter-2/dynamism.html
- https://github.com/goldbergyoni/nodebestpractices#-38-require-modules-first-not-inside-functions
**Incorrect (JavaScript - debug statements in code):**
Debug statements like `alert()`, `confirm()`, `prompt()`, and `debugger` should not be in production code.
```javascript
// ruleid:javascript-prompt
var name = prompt('what is your name');
// ruleid: javascript-alert
alert('your name is ' + name);
// ruleid: javascript-confirm
if ( confirm("pushem!") == true) {
r = "x";
} else {
r = "Y";
// ruleid: javascript-debugger
debugger;
const mod = require('module-name')
function smth() {
return mod();
}
```
**Incorrect (JavaScript - async zlib operations in loops):**
### Secure Temporary File Creation
Creating and using a large number of zlib objects simultaneously can cause significant memory fragmentation. Cache compression results or make operations synchronous to avoid duplication of effort.
File creation in shared tmp directories without proper APIs can lead to security vulnerabilities.
```javascript
const zlib = require('zlib');
**Incorrect (Python):**
const payload = Buffer.from('This is some data');
for (let i = 0; i < 30000; ++i) {
// ruleid: zlib-async-loop
zlib.deflate(payload, (err, buffer) => {});
}
[1,2,3].forEach((el) => {
// ruleid: zlib-async-loop
zlib.deflate(payload, (err, buffer) => {});
})
```python
with open('/tmp/myfile.txt', 'w') as f:
f.write(data)
```
**Correct (JavaScript - sync zlib or single async call):**
**Correct (Python):**
```javascript
for (let i = 0; i < 30000; ++i) {
// ok: zlib-async-loop
zlib.deflateSync(payload);
}
// ok: zlib-async-loop
zlib.deflate(payload, (err, buffer) => {});
```python
import tempfile
with tempfile.NamedTemporaryFile(mode='w', delete=False) as f:
f.write(data)
```
**References:**
- https://nodejs.org/api/zlib.html#zlib_threadpool_usage_and_performance_considerations
### Cookie Security Flags
**Incorrect (TypeScript - using deprecated Moment.js):**
Always set `HttpOnly` and `Secure` flags on security-sensitive cookies.
Moment.js is a legacy project in maintenance mode. Consider using actively supported libraries like `dayjs`.
**Incorrect (JavaScript/Express):**
```typescript
// ruleid: moment-deprecated
```javascript
res.cookie('session', value);
```
**Correct (JavaScript/Express):**
```javascript
res.cookie('session', value, { httpOnly: true, secure: true });
```
### Validate Redirect URLs
Never redirect to user-provided URLs without validation to prevent open redirect vulnerabilities.
**Incorrect (JavaScript):**
```javascript
res.redirect(req.query.returnUrl);
```
**Correct (JavaScript):**
```javascript
const allowedHosts = ['example.com'];
const url = new URL(req.query.returnUrl, 'https://example.com');
if (allowedHosts.includes(url.hostname)) {
res.redirect(url.href);
}
```
### Avoid Deprecated Libraries
Use actively maintained alternatives instead of deprecated libraries.
**Incorrect (JavaScript - Moment.js is deprecated):**
```javascript
import moment from 'moment';
// ruleid: moment-deprecated
import { moment } from 'moment';
```
**Correct (TypeScript - using dayjs):**
**Correct (JavaScript - use dayjs):**
```typescript
// ok: moment-deprecated
```javascript
import dayjs from 'dayjs';
```
**References:**
- https://momentjs.com/docs/#/-project-status/
- https://day.js.org/
**Incorrect (React - spreading props directly):**
Explicitly pass props to HTML components rather than using the spread operator. The spread operator risks passing invalid HTML props or allowing malicious attribute injection.
```jsx
function Test1(props) {
// ruleid: react-props-spreading
const el = <App {...props} />;
return el;
}
function Test2(props) {
// ruleid: react-props-spreading
const el = <MyCustomComponent {...props} some_other_prop={some_other_prop} />;
return el;
}
```
**Correct (React - explicit props):**
```jsx
function Test2(props, otherProps) {
const {src, alt} = props;
const {one_prop, two_prop} = otherProps;
// ok: react-props-spreading
return <MyCustomComponent one_prop={one_prop} two_prop={two_prop} />;
}
```
**References:**
- https://github.com/yannickcr/eslint-plugin-react/blob/master/docs/rules/jsx-props-no-spreading.md
**Incorrect (React - copying props into state):**
Copying a prop into state causes all updates to be ignored. Read props directly in your component instead.
```jsx
class Test1 extends React.Component {
constructor() {
// ruleid:react-props-in-state
this.state = {
foo: 'bar',
color: this.props.color,
one: 1
};
}
}
function Test3({ text }) {
// ruleid:react-props-in-state
const [buttonText] = useState(text)
return <button>{buttonText}</button>
}
```
**Correct (React - using props directly):**
```jsx
class OkTest extends React.Component {
// ok: react-props-in-state
constructor() {
this.state = {
foo: 'bar',
initialColor: this.props.color,
one: 1
};
}
}
function OkTest1({ color, children }) {
const textColor = useMemo(
// ok: react-props-in-state
() => slowlyCalculateTextColor(color),
[color]
);
}
```
**References:**
- https://overreacted.io/writing-resilient-components/#principle-1-dont-stop-the-data-flow
**Incorrect (Bash - iterating over ls output):**
Iterating over `ls` output is fragile. Use globs like `dir/*` instead.
```bash
# ruleid:iteration-over-ls-output
for file in $(ls dir); do echo "Found a file: $file"; done
# ruleid:iteration-over-ls-output
for file in $(ls dir)
do
echo "Found a file: $file"
done
```
**Correct (Bash - using globs):**
```bash
# ok:iteration-over-ls-output
for file in dir/*; do
echo "Found a file: $file"
done
```
**References:**
- https://github.com/koalaman/shellcheck/wiki/SC2045
**Incorrect (Bash - useless cat):**
Useless calls to `cat` in a pipeline waste resources. Use `<` and `>` for reading from or writing to files.
```bash
# ruleid:useless-cat
cat | a b
# ruleid:useless-cat
cat file | a b
# ruleid:useless-cat
a b | cat > file
# ruleid:useless-cat
a b | cat | c d
```
**Correct (Bash - efficient file operations):**
```bash
# ok:useless-cat
a b
# ok:useless-cat
cat file1 file2 | a b
# ok:useless-cat
cat $files | a b
```
**References:**
- https://github.com/koalaman/shellcheck/wiki/SC2002
**Incorrect (Java - bad hexadecimal conversion):**
`Integer.toHexString()` strips leading zeroes from each byte when read byte-by-byte. This weakens hash values by introducing more collisions. Use `String.format("%02X", ...)` instead.
```java
// ruleid: bad-hexa-conversion
public static String badHash(String password) throws NoSuchAlgorithmException, UnsupportedEncodingException {
MessageDigest md = MessageDigest.getInstance("SHA-1");
byte[] resultBytes = md.digest(password.getBytes("UTF-8"));
StringBuilder stringBuilder = new StringBuilder();
for (byte b : resultBytes) {
stringBuilder.append(Integer.toHexString(b & 0xFF));
}
return stringBuilder.toString();
}
```
**Correct (Java - proper hexadecimal conversion):**
```java
// ok: bad-hexa-conversion
public static String goodHash(String password) throws NoSuchAlgorithmException, UnsupportedEncodingException {
MessageDigest md = MessageDigest.getInstance("SHA-1");
byte[] resultBytes = md.digest(password.getBytes("UTF-8"));
StringBuilder stringBuilder = new StringBuilder();
for (byte b : resultBytes) {
stringBuilder.append(String.format("%02X", b));
}
return stringBuilder.toString();
}
```
**References:**
- https://find-sec-bugs.github.io/bugs.htm#BAD_HEXA_CONVERSION
**Incorrect (Kotlin - cookie missing HttpOnly flag):**
The `HttpOnly` flag instructs the browser to forbid client-side scripts from reading the cookie. Always set this flag for security-sensitive cookies.
```kotlin
public class CookieController {
public fun setCookie(value: String, response: HttpServletResponse) {
val cookie: Cookie = Cookie("cookie", value)
// ruleid: cookie-missing-httponly
response.addCookie(cookie)
}
public fun explicitDisable(value: String, response: HttpServletResponse) {
val cookie: Cookie = Cookie("cookie", value)
cookie.setSecure(false)
// ruleid:cookie-missing-httponly
cookie.setHttpOnly(false)
response.addCookie(cookie)
}
}
```
**Correct (Kotlin - cookie with HttpOnly flag):**
```kotlin
public fun setSecureHttponlyCookie(value: String, response: HttpServletResponse ) {
val cookie: Cookie = Cookie("cookie", value)
cookie.setSecure(true)
cookie.setHttpOnly(true)
// ok: cookie-missing-httponly
response.addCookie(cookie)
}
```
**References:**
- https://find-sec-bugs.github.io/bugs.htm#HTTPONLY_COOKIE
**Incorrect (C - using memset for sensitive data):**
When handling sensitive information in a buffer, `memset()` can leave sensitive information behind due to compiler optimizations. Use `memset_s()` which securely overwrites memory.
```c
void badcode(char *password, size_t bufferSize) {
char token[256];
init(token, password);
// ruleid: insecure-use-memset
memset(password, ' ', strlen(password));
// ruleid: insecure-use-memset
memset(token, ' ', strlen(localBuffer));
free(password);
}
```
**Correct (C - using memset_s for sensitive data):**
```c
void okcode(char *password, size_t bufferSize) {
char token[256];
init(token, password);
// ok: insecure-use-memset
memset_s(password, bufferSize, ' ', strlen(password));
// ok: insecure-use-memset
memset_s(token, sizeof(token), ' ', strlen(localBuffer));
free(password);
}
```
**References:**
- https://cwe.mitre.org/data/definitions/14.html
- https://owasp.org/Top10/A02_2021-Cryptographic_Failures/
**Incorrect (Go - bad tmp file creation):**
File creation in shared tmp directory without using `ioutil.Tempfile` can lead to insecure temporary file vulnerabilities.
```go
func main() {
// ruleid:bad-tmp-file-creation
err := ioutil.WriteFile("/tmp/demo2", []byte("This is some data"), 0644)
if err != nil {
fmt.Println("Error while writing!")
}
}
```
**Correct (Go - using ioutil.Tempfile):**
```go
func main_good() {
// ok:bad-tmp-file-creation
err := ioutil.Tempfile("/tmp", "my_temp")
if err != nil {
fmt.Println("Error while writing!")
}
}
```
**References:**
- https://owasp.org/Top10/A01_2021-Broken_Access_Control
**Incorrect (Rust - using temp_dir for security operations):**
`temp_dir()` should not be used for security operations as the temporary directory may be shared among users or processes with different privileges.
```rust
use std::env;
// ruleid: temp-dir
let dir = env::temp_dir();
```
**References:**
- https://doc.rust-lang.org/stable/std/env/fn.temp_dir.html
**Incorrect (Elixir - deprecated use Bitwise):**
The syntax `use Bitwise` is deprecated. Use `import Bitwise` instead.
```elixir
# ruleid: deprecated_use_bitwise
use Bitwise
```
**Correct (Elixir - import Bitwise):**
```elixir
import Bitwise
```
**References:**
- https://github.com/elixir-lang/elixir/commit/f1b9d3e818e5bebd44540f87be85979f24b9abfc
**Incorrect (Elixir - inefficient Enum.map then Enum.join):**
Using `Enum.map_join/3` is more efficient than `Enum.map/2 |> Enum.join/2`.
```elixir
# ruleid: enum_map_join
Enum.join(Enum.map(["a", "b", "c"], fn s -> String.upcase(s) end), ", ")
# ruleid: enum_map_join
Enum.map(["a", "b", "c"], fn s -> String.upcase(s) end)
|> Enum.join(", ")
# ruleid: enum_map_join
["a", "b", "c"]
|> Enum.map(fn s -> String.upcase(s) end)
|> Enum.join(", ")
```
**References:**
- https://github.com/rrrene/credo/blob/master/lib/credo/check/refactor/map_join.ex
**Incorrect (OCaml - explicit boolean comparisons):**
Comparing to `true` or `false` explicitly is unnecessary and reduces readability.
```ocaml
let test a =
(* ruleid:ocamllint-bool-true *)
let x = a = true in
(* ruleid:ocamllint-bool-true *)
let x = a == true in
(* ruleid:ocamllint-bool-false *)
let x = a = false in
(* ruleid:ocamllint-bool-false *)
let x = a == false in
()
```
**Correct (OCaml - implicit boolean evaluation):**
Use `$X` directly instead of `$X = true`, and `not $X` instead of `$X = false`.
**Incorrect (OCaml - List.find outside try block):**
`List.find` should be used inside a try block, or use `List.find_opt` instead.
```ocaml
let test1 xs =
(* ruleid:list-find-outside-try *)
if List.find 1 xs
then 1
else 2
```
**Correct (OCaml - List.find inside try block):**
```ocaml
let test2 xs =
(* ok *)
try
if List.find 1 xs
then 1
else 2
with Not_found -> 3
```
**Incorrect (Ruby - unscoped find with user input):**
Unscoped `find(...)` with user-controllable input may lead to Insecure Direct Object Reference (IDOR) behavior.
```ruby
class GroupsController < ApplicationController
def show
#ruleid: check-unscoped-find
@user = User.find(params[:id])
end
def get
#ruleid: check-unscoped-find
@some_record = SomeRecord.find_by_id!(params[:id])
end
end
```
**Correct (Ruby - scoped find operations):**
```ruby
def show_ok
#ok: check-unscoped-find
@user = User.find(session[:id])
end
def show_ok2
#ok: check-unscoped-find
current_user = User.find(session[:id])
#ok: check-unscoped-find
current_user.accounts.find(param[:id])
end
```
**References:**
- https://brakemanscanner.org/docs/warning_types/unscoped_find/
**Incorrect (PHP - phpinfo in production):**
The `phpinfo` function may reveal sensitive information about your environment.
```php
<?php
// ruleid: phpinfo-use
echo phpinfo();
```
**References:**
- https://www.php.net/manual/en/function.phpinfo
**Incorrect (Swift - sensitive data in UserDefaults):**
Sensitive data stored in UserDefaults is not adequately protected. Use the Keychain for data of a sensitive nature.
```swift
let passphrase = getPass()
// ruleid: swift-user-defaults
UserDefaults.standard.set(passphrase, forKey: "passphrase")
// ruleid: swift-user-defaults
UserDefaults.standard.set(passWord, forKey: "userPassword")
// ruleid: swift-user-defaults
UserDefaults.standard.set("12717-127163-a71367-127ahc", forKey: "apiKey")
let apiKey = "12717-127163-a71367-127ahc"
// ruleid: swift-user-defaults
UserDefaults.standard.set(apiKey, forKey: "GOOGLE_TOKEN")
```
**Correct (Swift - non-sensitive data in UserDefaults):**
```swift
let username = getUsername()
// okid: swift-user-defaults
UserDefaults.standard.set(username, forKey: "userName")
```
**References:**
- https://developer.apple.com/library/archive/documentation/Security/Conceptual/SecureCodingGuide/Articles/ValidatingInput.html
- https://mas.owasp.org/MASVS/controls/MASVS-STORAGE-1/
**Incorrect (Terraform - S3 bucket with public read access):**
S3 buckets with public read access expose data to unauthorized users.
```hcl
resource "aws_s3_bucket" "a" {
bucket = "my-tf-test-bucket"
# ruleid: s3-public-read-bucket
acl = "public-read"
tags = {
Name = "My bucket"
Environment = "Dev"
}
}
resource "aws_s3_bucket" "b" {
bucket = "my-tf-test-bucket-b"
# ruleid: s3-public-read-bucket
acl = "authenticated-read"
}
```
**Correct (Terraform - S3 bucket with policy):**
```hcl
resource "aws_s3_bucket" "c" {
bucket = "s3-website-test.hashicorp.com"
# ok: s3-public-read-bucket
acl = "public-read"
policy = file("policy.json")
website {
index_document = "index.html"
error_document = "error.html"
}
}
```
**References:**
- https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/s3_bucket#acl
- https://docs.aws.amazon.com/AmazonS3/latest/dev/acl-overview.html#canned-acl
**Incorrect (C# - open redirect vulnerability):**
A query string parameter may contain a URL value that could cause the web application to redirect to a malicious website. Always validate redirect URLs.
```csharp
[HttpPost]
public ActionResult LogOn(LogOnModel model, string returnUrl)
{
if (ModelState.IsValid)
{
if (MembershipService.ValidateUser(model.UserName, model.Password))
{
FormsService.SignIn(model.UserName, model.RememberMe);
if (!String.IsNullOrEmpty(returnUrl))
{
// ruleid: open-redirect
return Redirect(returnUrl);
}
}
}
}
```
**Correct (C# - validated redirect URL):**
```csharp
[HttpPost]
public ActionResult LogOn(LogOnModel model, string returnUrl)
{
if (ModelState.IsValid)
{
if (MembershipService.ValidateUser(model.UserName, model.Password))
{
FormsService.SignIn(model.UserName, model.RememberMe);
if (IsLocalUrl(returnUrl))
{
// ok: open-redirect
return Redirect(returnUrl);
}
else
{
return RedirectToAction("Index", "Home");
}
}
}
}
```
**References:**
- https://cwe.mitre.org/data/definitions/601.html
skills/code-security/rules/code-injection.md
+23 -1173
View File
File diff suppressed because it is too large Load Diff
skills/code-security/rules/command-injection.md
+62 -860
View File
File diff suppressed because it is too large Load Diff
skills/code-security/rules/correctness.md
+67 -995
View File
File diff suppressed because it is too large Load Diff
skills/code-security/rules/csrf.md
+63 -836
View File
@@ -7,361 +7,7 @@ tags: security, csrf, cwe-352, owasp-a01
## Prevent Cross-Site Request Forgery
Cross-Site Request Forgery (CSRF) is an attack that forces authenticated users to execute unwanted actions on a web application. When a user is authenticated, their browser automatically includes session cookies with requests. Attackers can craft malicious pages that trigger requests to vulnerable applications, causing actions to be performed without the user's consent. CSRF attacks can result in unauthorized fund transfers, email address changes, password changes, or any other state-changing operation.
---
### Language: Ruby / Rails
#### Skip Forgery Protection
**Incorrect (disables CSRF protection entirely):**
```ruby
class CustomStrategy
def initialize(controller)
@controller = controller
end
def handle_unverified_request
# Custom behaviour for unverfied request
end
end
class ApplicationController < ActionController::Base
# ruleid: rails-skip-forgery-protection
skip_forgery_protection
end
```
**Correct (CSRF protection enabled by default):**
```ruby
class ApplicationController2 < ActionController::Base
# ok: rails-skip-forgery-protection
end
```
**References:**
- [Rails ActionController RequestForgeryProtection](https://api.rubyonrails.org/classes/ActionController/RequestForgeryProtection/ClassMethods.html#method-i-skip_forgery_protection)
---
#### Missing CSRF Protection
**Incorrect (controller without protect_from_forgery):**
```ruby
# ruleid:missing-csrf-protection
class DangerousController < ActionController::Base
puts "do more stuff"
end
```
**Correct (controller with protect_from_forgery):**
```ruby
# ok:missing-csrf-protection
class OkController < ActionController::Base
protect_from_forgery :with => :exception
puts "do more stuff"
end
# ok:missing-csrf-protection
class OkController < ActionController::Base
protect_from_forgery prepend: true, with: :exception
puts "do more stuff"
end
```
**References:**
- [OWASP Top 10 A01:2021 - Broken Access Control](https://owasp.org/Top10/A01_2021-Broken_Access_Control)
---
### Language: JavaScript / Express
#### CSRF Before Method Override
**Incorrect (csrf() before methodOverride() allows bypass):**
```javascript
function bad() {
// ruleid:detect-no-csrf-before-method-override
express.csrf()
express.methodOverride()
}
```
**Correct (methodOverride() before csrf()):**
```javascript
function ok() {
// ok:detect-no-csrf-before-method-override
express.methodOverride()
express.csrf()
}
```
**References:**
- [Bypass Connect CSRF Protection by Abusing Method Override](https://github.com/nodesecurity/eslint-plugin-security/blob/master/docs/bypass-connect-csrf-protection-by-abusing.md)
---
#### Missing CSRF Middleware in Express
**Incorrect (Express app without csurf middleware):**
```javascript
var cookieParser = require('cookie-parser') //for cookie parsing
// var csrf = require('csurf') //csrf module
var bodyParser = require('body-parser') //for body parsing
var express = require('express')
// setup route middlewares
var csrfProtection = csrf({
cookie: true
})
var parseForm = bodyParser.urlencoded({
extended: false
})
// ruleid: express-check-csurf-middleware-usage
var app = express()
// parse cookies
app.use(cookieParser())
app.get('/form', csrfProtection, function(req, res) {
// generate and pass the csrfToken to the view
res.render('send', {
csrfToken: req.csrfToken()
})
})
app.post('/process', parseForm, csrfProtection, function(req, res) {
res.send('data is being processed')
})
app.post('/bad', parseForm, function(req, res) {
res.send('data is being processed')
})
```
**Correct (include csurf or csrf middleware):**
```javascript
var csrf = require('csurf')
var express = require('express')
// ok: express-check-csurf-middleware-usage
var app = express()
app.use(csrf({ cookie: true }))
```
**References:**
- [csurf npm package](https://www.npmjs.com/package/csurf)
- [csrf npm package](https://www.npmjs.com/package/csrf)
- [OWASP CSRF Prevention Cheat Sheet](https://cheatsheetseries.owasp.org/cheatsheets/Cross-Site_Request_Forgery_Prevention_Cheat_Sheet.html)
---
### Language: C# / ASP.NET MVC
#### Missing Anti-Forgery Token Validation
**Incorrect (state-changing methods without ValidateAntiForgeryToken):**
```csharp
using System.Diagnostics;
using Microsoft.AspNetCore.Mvc;
using MvcMovie.Models;
public class HomeController : Controller
{
private readonly ILogger<HomeController> _logger;
public HomeController(ILogger<HomeController> logger)
{
_logger = logger;
}
public IActionResult Index()
{
return View();
}
//ruleid: mvc-missing-antiforgery
[HttpPost]
public IActionResult CreateBad(User user){
CreateUser(user);
}
//ruleid: mvc-missing-antiforgery
[HttpDelete]
public IActionResult DeleteBad(User user){
DeleteUser(user);
}
}
```
**Correct (add ValidateAntiForgeryToken or strict Content-Type checking):**
```csharp
using System.Diagnostics;
using Microsoft.AspNetCore.Mvc;
using MvcMovie.Models;
public class HomeController : Controller
{
//ok: mvc-missing-antiforgery
[HttpPost]
[ValidateAntiForgeryToken]
public IActionResult CreateGood(User user){
CreateUser(user);
}
//ok: mvc-missing-antiforgery
[HttpPost]
//strict type checking enforces CORS preflight for non-simple HTTP requests
[Consumes("application/json")]
public IActionResult CreateGood(User user){
CreateUser(user);
}
//ok: mvc-missing-antiforgery
[ValidateAntiForgeryToken]
[HttpDelete]
public IActionResult DeleteGood(User user){
CreateUser(user);
}
}
```
**References:**
- [.NET Security Cheat Sheet - CSRF](https://cheatsheetseries.owasp.org/cheatsheets/DotNet_Security_Cheat_Sheet.html#cross-site-request-forgery)
- [MDN CORS Simple Requests](https://developer.mozilla.org/en-US/docs/Web/HTTP/CORS#simple_requests)
---
### Language: Java / Spring
#### Unrestricted Request Mapping
**Incorrect (RequestMapping without specifying HTTP method):**
```java
// cf. https://find-sec-bugs.github.io/bugs.htm#SPRING_CSRF_UNRESTRICTED_REQUEST_MAPPING
@Controller
public class Controller {
// ruleid: unrestricted-request-mapping
@RequestMapping("/path")
public void writeData() {
// State-changing operations performed within this method.
}
// ruleid: unrestricted-request-mapping
@RequestMapping(value = "/path")
public void writeData2() {
// State-changing operations performed within this method.
}
}
```
**Correct (specify HTTP method in RequestMapping):**
```java
@Controller
public class Controller {
/**
* For methods without side-effects use either
* RequestMethod.GET, RequestMethod.HEAD, RequestMethod.TRACE, or RequestMethod.OPTIONS.
*/
// ok: unrestricted-request-mapping
@RequestMapping(value = "/path", method = RequestMethod.GET)
public String readData() {
// No state-changing operations performed within this method.
return "";
}
/**
* For state-changing methods use either
* RequestMethod.POST, RequestMethod.PUT, RequestMethod.DELETE, or RequestMethod.PATCH.
*/
// ok: unrestricted-request-mapping
@RequestMapping(value = "/path", method = RequestMethod.POST)
public void writeData3() {
// State-changing operations performed within this method.
}
}
```
**References:**
- [Find Security Bugs - Spring CSRF Unrestricted Request Mapping](https://find-sec-bugs.github.io/bugs.htm#SPRING_CSRF_UNRESTRICTED_REQUEST_MAPPING)
---
#### Spring CSRF Disabled
**Incorrect (explicitly disabling CSRF protection):**
```java
package com.example.securingweb; // cf. https://spring.io/guides/gs/securing-web/
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
import org.springframework.security.config.annotation.web.builders.HttpSecurity;
import org.springframework.security.config.annotation.web.configuration.EnableWebSecurity;
import org.springframework.security.config.annotation.web.configuration.WebSecurityConfigurerAdapter;
import org.springframework.security.core.userdetails.User;
import org.springframework.security.core.userdetails.UserDetails;
import org.springframework.security.core.userdetails.UserDetailsService;
import org.springframework.security.provisioning.InMemoryUserDetailsManager;
@Configuration
@EnableWebSecurity
public class WebSecurityConfigCsrfDisable extends WebSecurityConfigurerAdapter {
@Override
protected void configure(HttpSecurity http) throws Exception {
// ruleid: spring-csrf-disabled
http
.csrf().disable()
.authorizeRequests()
.antMatchers("/", "/home").permitAll()
.anyRequest().authenticated()
.and()
.formLogin()
.loginPage("/login")
.permitAll()
.and()
.logout()
.permitAll();
}
}
```
**Correct (CSRF protection enabled by default):**
```java
public class WebSecurityConfigOK extends WebSecurityConfigurerAdapter {
@Override
protected void configure(HttpSecurity http) throws Exception {
// ok: spring-csrf-disabled
http
.authorizeRequests()
.antMatchers("/", "/home").permitAll()
.anyRequest().authenticated()
.and()
.formLogin()
.loginPage("/login")
.permitAll()
.and()
.logout()
.permitAll();
}
}
```
**References:**
- [OWASP Top 10 A01:2021 - Broken Access Control](https://owasp.org/Top10/A01_2021-Broken_Access_Control)
Cross-Site Request Forgery (CSRF) is an attack that forces authenticated users to execute unwanted actions on a web application. When a user is authenticated, their browser automatically includes session cookies with requests. Attackers can craft malicious pages that trigger requests to vulnerable applications, causing actions to be performed without the user's consent.
---
@@ -374,24 +20,15 @@ public class WebSecurityConfigOK extends WebSecurityConfigurerAdapter {
from django.http import HttpResponse
from django.views.decorators.csrf import csrf_exempt
# ruleid: no-csrf-exempt
@csrf_exempt
def my_view(request):
return HttpResponse('Hello world')
import django
# ruleid: no-csrf-exempt
@django.views.decorators.csrf.csrf_exempt
def my_view2(request):
return HttpResponse('Hello world')
```
**Correct (remove csrf_exempt decorator):**
```python
from django.http import HttpResponse
# ok: no-csrf-exempt
def my_view(request):
return HttpResponse('Hello world')
```
@@ -401,508 +38,99 @@ def my_view(request):
---
### Language: Python / Pyramid
### Language: JavaScript / Express
#### CSRF Check Disabled Globally
#### Missing CSRF Middleware
**Incorrect (disabling CSRF checks globally):**
```python
from pyramid.csrf import CookieCSRFStoragePolicy
**Incorrect (Express app without csurf middleware):**
```javascript
var express = require('express')
var bodyParser = require('body-parser')
var app = express()
def includeme_bad(config):
config.set_csrf_storage_policy(CookieCSRFStoragePolicy())
# ruleid: pyramid-csrf-check-disabled-globally
config.set_default_csrf_options(require_csrf=False)
app.post('/process', bodyParser.urlencoded({ extended: false }), function(req, res) {
res.send('data is being processed')
})
```
**Correct (enable CSRF checks):**
```python
from pyramid.csrf import CookieCSRFStoragePolicy
**Correct (include csurf middleware):**
```javascript
var csrf = require('csurf')
var express = require('express')
def includeme_good(config):
config.set_csrf_storage_policy(CookieCSRFStoragePolicy())
# ok: pyramid-csrf-check-disabled-globally
config.set_default_csrf_options(require_csrf=True)
var app = express()
app.use(csrf({ cookie: true }))
```
**References:**
- [OWASP Top 10 A01:2021 - Broken Access Control](https://owasp.org/Top10/A01_2021-Broken_Access_Control)
- [csurf npm package](https://www.npmjs.com/package/csurf)
- [OWASP CSRF Prevention Cheat Sheet](https://cheatsheetseries.owasp.org/cheatsheets/Cross-Site_Request_Forgery_Prevention_Cheat_Sheet.html)
---
#### CSRF Check Disabled Per View
### Language: Java / Spring
**Incorrect (disabling CSRF for specific view):**
```python
from pyramid.view import view_config
#### CSRF Disabled
@view_config(
route_name='home_bad1',
# ruleid: pyramid-csrf-check-disabled
require_csrf=False,
renderer='my_app:templates/mytemplate.jinja2'
)
def my_bad_home1(request):
try:
query = request.dbsession.query(models.MyModel)
one = query.filter(models.MyModel.name == 'one').one()
except SQLAlchemyError:
return Response("Database error", content_type='text/plain', status=500)
return {'one': one, 'project': 'my_proj'}
**Incorrect (explicitly disabling CSRF protection):**
```java
@Configuration
@EnableWebSecurity
public class WebSecurityConfig extends WebSecurityConfigurerAdapter {
@Override
protected void configure(HttpSecurity http) throws Exception {
http
.csrf().disable()
.authorizeRequests()
.antMatchers("/", "/home").permitAll()
.anyRequest().authenticated();
}
}
```
**Correct (enable CSRF for view):**
```python
from pyramid.view import view_config
@view_config(
route_name='home_bad1',
# ok: pyramid-csrf-check-disabled
require_csrf=True,
renderer='my_app:templates/mytemplate.jinja2'
)
def my_good_home1(request):
try:
query = request.dbsession.query(models.MyModel)
one = query.filter(models.MyModel.name == 'one').one()
except SQLAlchemyError:
return Response("Database error", content_type='text/plain', status=500)
return {'one': one, 'project': 'my_proj'}
**Correct (CSRF protection enabled by default):**
```java
@Configuration
@EnableWebSecurity
public class WebSecurityConfig extends WebSecurityConfigurerAdapter {
@Override
protected void configure(HttpSecurity http) throws Exception {
http
.authorizeRequests()
.antMatchers("/", "/home").permitAll()
.anyRequest().authenticated();
}
}
```
**References:**
- [OWASP Top 10 A01:2021 - Broken Access Control](https://owasp.org/Top10/A01_2021-Broken_Access_Control)
- [Find Security Bugs - Spring CSRF](https://find-sec-bugs.github.io/bugs.htm#SPRING_CSRF_UNRESTRICTED_REQUEST_MAPPING)
---
#### CSRF Origin Check Disabled
### Language: Ruby / Rails
**Incorrect (disabling origin check for CSRF token):**
```python
from pyramid.view import view_config
#### Missing CSRF Protection
@view_config(
route_name='home_bad1',
# ruleid: pyramid-csrf-origin-check-disabled
check_origin=False,
renderer='my_app:templates/mytemplate.jinja2'
)
def my_bad_home1(request):
try:
query = request.dbsession.query(models.MyModel)
one = query.filter(models.MyModel.name == 'one').one()
except SQLAlchemyError:
return Response("Database error", content_type='text/plain', status=500)
return {'one': one, 'project': 'my_proj'}
**Incorrect (controller without protect_from_forgery):**
```ruby
class DangerousController < ActionController::Base
puts "do more stuff"
end
```
**Correct (enable origin check):**
```python
from pyramid.view import view_config
**Correct (controller with protect_from_forgery):**
```ruby
class SafeController < ActionController::Base
protect_from_forgery with: :exception
@view_config(
route_name='home_bad1',
# ok: pyramid-csrf-origin-check-disabled
check_origin=True,
renderer='my_app:templates/mytemplate.jinja2'
)
def my_good_home1(request):
try:
query = request.dbsession.query(models.MyModel)
one = query.filter(models.MyModel.name == 'one').one()
except SQLAlchemyError:
return Response("Database error", content_type='text/plain', status=500)
return {'one': one, 'project': 'my_proj'}
puts "do more stuff"
end
```
**References:**
- [OWASP Top 10 A01:2021 - Broken Access Control](https://owasp.org/Top10/A01_2021-Broken_Access_Control)
---
#### CSRF Origin Check Disabled Globally
**Incorrect (disabling origin check globally):**
```python
from pyramid.csrf import CookieCSRFStoragePolicy
def includeme_bad(config):
config.set_csrf_storage_policy(CookieCSRFStoragePolicy())
# ruleid: pyramid-csrf-origin-check-disabled-globally
config.set_default_csrf_options(check_origin=False)
```
**Correct (enable origin check globally):**
```python
from pyramid.csrf import CookieCSRFStoragePolicy
def includeme_good(config):
config.set_csrf_storage_policy(CookieCSRFStoragePolicy())
# ok: pyramid-csrf-origin-check-disabled-globally
config.set_default_csrf_options(check_origin=True)
```
**References:**
- [OWASP Top 10 A01:2021 - Broken Access Control](https://owasp.org/Top10/A01_2021-Broken_Access_Control)
---
### Language: Python / Flask
#### Flask-WTF CSRF Disabled
**Incorrect (disabling WTF_CSRF_ENABLED):**
```python
import flask
from flask import response as r
app = flask.Flask(__name__)
# ruleid:flask-wtf-csrf-disabled
app.config['WTF_CSRF_ENABLED'] = False
# ruleid:flask-wtf-csrf-disabled
app.config["WTF_CSRF_ENABLED"] = False
# ruleid: flask-wtf-csrf-disabled
app.config.WTF_CSRF_ENABLED = False
# DICT UPDATE
################
app.config.update(
SECRET_KEY='192b9bdd22ab9ed4d12e236c78afcb9a393ec15f71bbf5dc987d54727823bcbf',
# ruleid: flask-wtf-csrf-disabled
WTF_CSRF_ENABLED = False,
TESTING=False
)
# FROM OBJECT
################
# custom class
appconfig = MyAppConfig()
# ruleid: flask-wtf-csrf-disabled
appconfig.WTF_CSRF_ENABLED = False
app.config.from_object(appconfig)
# this file itself
SECRET_KEY = 'development key'
# ruleid: flask-wtf-csrf-disabled
WTF_CSRF_ENABLED = False
app.config.from_object(__name__)
# FROM MAPPING
################
app.config.from_mapping(
SECRET_KEY='192b9bdd22ab9ed4d12e236c78afcb9a393ec15f71bbf5dc987d54727823bcbf',
# ruleid: flask-wtf-csrf-disabled
WTF_CSRF_ENABLED = False,
)
```
**Correct (enable CSRF or only disable for testing):**
```python
import flask
app = flask.Flask(__name__)
# ok: flask-wtf-csrf-disabled
app.config["WTF_CSRF_ENABLED"] = True
# ok: flask-wtf-csrf-disabled
app.config["SESSION_COOKIE_SECURE"] = False
# ok: flask-wtf-csrf-disabled
app.config.WTF_CSRF_ENABLED = True
# It's okay to do this during testing
app.config.update(
SECRET_KEY='192b9bdd22ab9ed4d12e236c78afcb9a393ec15f71bbf5dc987d54727823bcbf',
# ok: flask-wtf-csrf-disabled
WTF_CSRF_ENABLED = False,
TESTING=True
)
# It's okay to do this during testing
app.config.from_mapping(
SECRET_KEY='192b9bdd22ab9ed4d12e236c78afcb9a393ec15f71bbf5dc987d54727823bcbf',
# ok: flask-wtf-csrf-disabled
WTF_CSRF_ENABLED = False,
TESTING=True
)
```
**References:**
- [Flask-WTF CSRF Protection](https://flask-wtf.readthedocs.io/en/1.2.x/csrf/)
---
### Language: PHP / Symfony
#### Symfony CSRF Protection Disabled
**Incorrect (disabling csrf_protection in forms or configuration):**
```php
<?php
use Symfony\Component\Form\AbstractType;
use Symfony\Component\OptionsResolver\OptionsResolver;
use Symfony\Component\HttpKernel\DependencyInjection\Extension;
use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\DependencyInjection\Extension\PrependExtensionInterface;
use Symfony\Bundle\FrameworkBundle\Controller\AbstractController;
use Symfony\Bundle\FrameworkBundle\Controller\Controller;
class Type extends AbstractType
{
public function configureOptions(OptionsResolver $resolver)
{
// ruleid: symfony-csrf-protection-disabled
$resolver->setDefaults([
'data_class' => Type::class,
'csrf_protection' => false
]);
// ruleid: symfony-csrf-protection-disabled
$resolver->setDefaults(array(
'csrf_protection' => false
));
$csrf = false;
// ruleid: symfony-csrf-protection-disabled
$resolver->setDefaults([
'csrf_protection' => $csrf
]);
}
}
class TestExtension extends Extension implements PrependExtensionInterface
{
public function prepend(ContainerBuilder $container)
{
// ruleid: symfony-csrf-protection-disabled
$container->prependExtensionConfig('framework', ['csrf_protection' => false,]);
// ruleid: symfony-csrf-protection-disabled
$container->prependExtensionConfig('framework', ['something_else' => true, 'csrf_protection' => false,]);
$csrfOption = false;
// ruleid: symfony-csrf-protection-disabled
$container->prependExtensionConfig('framework', ['csrf_protection' => $csrfOption,]);
// ruleid: symfony-csrf-protection-disabled
$container->loadFromExtension('framework', ['csrf_protection' => false,]);
}
}
class MyController1 extends AbstractController
{
public function action()
{
// ruleid: symfony-csrf-protection-disabled
$this->createForm(TaskType::class, $task, [
'other_option' => false,
'csrf_protection' => false,
]);
// ruleid: symfony-csrf-protection-disabled
$this->createForm(TaskType::class, $task, array(
'csrf_protection' => false,
));
$csrf = false;
// ruleid: symfony-csrf-protection-disabled
$this->createForm(TaskType::class, $task, array(
'csrf_protection' => $csrf,
));
}
}
```
**Correct (enable CSRF protection):**
```php
<?php
use Symfony\Component\Form\AbstractType;
use Symfony\Component\OptionsResolver\OptionsResolver;
class Type extends AbstractType
{
public function configureOptions(OptionsResolver $resolver)
{
// ok: symfony-csrf-protection-disabled
$resolver->setDefaults([
'csrf_protection' => true
]);
// ok: symfony-csrf-protection-disabled
$resolver->setDefaults([
'data_class' => Type::class,
]);
// ok: symfony-csrf-protection-disabled
$resolver->setDefaults($options);
}
}
class TestExtension extends Extension implements PrependExtensionInterface
{
public function prepend(ContainerBuilder $container)
{
// ok: symfony-csrf-protection-disabled
$container->loadFromExtension('framework', ['csrf_protection' => null,]);
// ok: symfony-csrf-protection-disabled
$container->prependExtensionConfig('framework', ['csrf_protection' => true,]);
// ok: symfony-csrf-protection-disabled
$container->prependExtensionConfig('framework', ['csrf_protection' => null,]);
// ok: symfony-csrf-protection-disabled
$container->prependExtensionConfig('something_else', ['csrf_protection' => false,]);
}
}
class MyController1 extends AbstractController
{
public function action()
{
// ok: symfony-csrf-protection-disabled
$this->createForm(TaskType::class, $task, ['csrf_protection' => true]);
// ok: symfony-csrf-protection-disabled
$this->createForm(TaskType::class, $task, ['other_option' => false]);
}
}
```
**References:**
- [Symfony CSRF Protection](https://symfony.com/doc/current/security/csrf.html)
---
### Language: PHP / WordPress
#### WordPress CSRF Audit - Useless Check
**Incorrect (check_ajax_referer with false third argument):**
```php
<?php
// ruleid: wp-csrf-audit
check_ajax_referer( 'wpforms-admin', 'nonce', false );
```
**Correct (check_ajax_referer with die enabled):**
```php
<?php
// ok: wp-csrf-audit
check_ajax_referer( 'wpforms-admin', 'nonce', true );
// ok: wp-csrf-audit
check_ajax_referer( 'wpforms-admin', 'nonce' );
?>
```
**References:**
- [WordPress CSRF Security Testing Cheat Sheet](https://github.com/wpscanteam/wpscan/wiki/WordPress-Plugin-Security-Testing-Cheat-Sheet#cross-site-request-forgery-csrf)
- [WordPress check_ajax_referer Reference](https://developer.wordpress.org/reference/functions/check_ajax_referer/)
---
### Language: Go / Gorilla WebSocket
#### WebSocket Missing Origin Check
**Incorrect (WebSocket upgrade without CheckOrigin):**
```go
package main
import (
"log"
"net/http"
"github.com/gorilla/websocket"
)
var upgrader2 = websocket.Upgrader{
ReadBufferSize: 1024,
WriteBufferSize: 1024,
}
func handler_doesnt_check_origin(w http.ResponseWriter, r *http.Request) {
// ruleid: websocket-missing-origin-check
conn, err := upgrader2.Upgrade(w, r, nil)
if err != nil {
log.Println(err)
return
}
}
```
**Correct (WebSocket upgrade with CheckOrigin):**
```go
package main
import (
"log"
"net/http"
"github.com/gorilla/websocket"
)
var upgrader = websocket.Upgrader{
CheckOrigin: func(r *http.Request) bool {
return true
},
ReadBufferSize: 1024,
WriteBufferSize: 1024,
}
var upgrader2 = websocket.Upgrader{
ReadBufferSize: 1024,
WriteBufferSize: 1024,
}
func handler_check_origin(w http.ResponseWriter, r *http.Request) {
// ok: websocket-missing-origin-check
conn, err := upgrader.Upgrade(w, r, nil)
if err != nil {
log.Println(err)
return
}
}
func handler_check_origin2(w http.ResponseWriter, r *http.Request) {
upgrader2.CheckOrigin = func(r *http.Request) bool { return true }
// ok: websocket-missing-origin-check
conn, err := upgrader2.Upgrade(w, r, nil)
if err != nil {
log.Println(err)
return
}
}
```
**References:**
- [Gorilla WebSocket Upgrader Documentation](https://pkg.go.dev/github.com/gorilla/websocket#Upgrader)
- [Rails ActionController RequestForgeryProtection](https://api.rubyonrails.org/classes/ActionController/RequestForgeryProtection/ClassMethods.html)
---
@@ -910,4 +138,3 @@ func handler_check_origin2(w http.ResponseWriter, r *http.Request) {
- CWE-352: Cross-Site Request Forgery (CSRF)
- [OWASP Top 10 A01:2021 - Broken Access Control](https://owasp.org/Top10/A01_2021-Broken_Access_Control)
- [OWASP CSRF Prevention Cheat Sheet](https://cheatsheetseries.owasp.org/cheatsheets/Cross-Site_Request_Forgery_Prevention_Cheat_Sheet.html)
- [OWASP CSRF Attack Description](https://owasp.org/www-community/attacks/csrf)
skills/code-security/rules/docker.md
+37 -385
View File
@@ -7,478 +7,130 @@ impact: HIGH
This guide provides security best practices for Dockerfiles and docker-compose configurations. Following these patterns helps prevent container escapes, privilege escalation, and other security vulnerabilities in containerized environments.
**Incorrect (Dockerfile - last user is root):**
### Running as Root
The last user in the container should not be 'root'. If an attacker gains control of the container, they will have root access.
**Incorrect:**
```dockerfile
FROM busybox
RUN git clone https://github.com/returntocorp/semgrep
RUN pip3 install semgrep
RUN semgrep -f p/xss
USER swuser
USER root
USER user1
# ruleid: last-user-is-root
RUN apt-get update && apt-get install -y some-package
USER appuser
USER root
```
**Correct (Dockerfile - last user is non-root):**
**Correct:**
```dockerfile
FROM busybox
RUN git clone https://github.com/returntocorp/semgrep
RUN pip3 install semgrep
USER root
RUN apt-get update && apt-get install -y some-package
USER appuser
```
**Incorrect (Dockerfile - missing image version):**
### Missing Image Version
Images should be tagged with an explicit version to produce deterministic container builds.
**Incorrect:**
```dockerfile
# ruleid: missing-image-version
FROM debian
# ruleid: missing-image-version
FROM nixos/nix
# ruleid: missing-image-version
FROM debian AS blah
# ruleid: missing-image-version
FROM nixos/nix AS build
# ruleid: missing-image-version
FROM --platform=linux/amd64 debian
# ruleid: missing-image-version
FROM --platform=linux/amd64 debian as name
```
**Correct (Dockerfile - explicit image version):**
**Correct:**
```dockerfile
# ok: missing-image-version
FROM debian:jessie
# ok: missing-image-version
FROM nixos/nix:2.7.0
# ok: missing-image-version
FROM debian:jessie AS blah
# ok: missing-image-version
FROM nixos/nix:2.7.0 AS build
# ok: missing-image-version
FROM --platform=linux/amd64 debian:jessie
# ok: missing-image-version
FROM --platform=linux/amd64 debian:jessie as name
# ok: missing-image-version
FROM python:3.10.1-alpine3.15@sha256:4be65b406f7402b5c4fd5df7173d2fd7ea3fdaa74d9c43b6ebd896197a45c448
# ok: missing-image-version
FROM python@sha256:4be65b406f7402b5c4fd5df7173d2fd7ea3fdaa74d9c43b6ebd896197a45c448
# ok: missing-image-version
FROM scratch
FROM debian:bookworm
```
**Incorrect (Dockerfile - using latest tag):**
### Using Latest Tag
The 'latest' tag may change the base container without warning, producing non-deterministic builds.
**Incorrect:**
```dockerfile
# ruleid: avoid-latest-version
FROM debian:latest
# ruleid: avoid-latest-version
FROM myregistry.local/testing/test-image:latest
# ruleid: avoid-latest-version
FROM debian:latest as blah
# ruleid: avoid-latest-version
FROM myregistry.local/testing/test-image:latest as blah
```
**Correct (Dockerfile - specific version tag):**
**Correct:**
```dockerfile
# ok: avoid-latest-version
FROM debian:jessie
# ok: avoid-latest-version
FROM myregistry.local/testing/test-image:42ee222
# ok: avoid-latest-version
FROM debian:jessie as blah2
# ok: avoid-latest-version
FROM myregistry.local/testing/test-image:2a4af68 as blah2
FROM debian:bookworm
```
**Incorrect (Dockerfile - relative WORKDIR):**
Use absolute paths for WORKDIR to prevent issues based on assumptions about the WORKDIR of previous containers.
```dockerfile
FROM busybox
# ruleid: use-absolute-workdir
WORKDIR usr/src/app
ENV dirpath=bar
# ruleid: use-absolute-workdir
WORKDIR ${dirpath}
```
**Correct (Dockerfile - absolute WORKDIR):**
```dockerfile
FROM busybox
# ok: use-absolute-workdir
WORKDIR /usr/src/app
ENV dirpath=/bar
# ok: use-absolute-workdir
WORKDIR ${dirpath}
```
**Incorrect (Dockerfile - using ADD for remote files):**
ADD will accept and include files from URLs and automatically extract archives. This potentially exposes the container to man-in-the-middle attacks. Use COPY instead for local files.
```dockerfile
FROM busybox
# ruleid: prefer-copy-over-add
ADD http://foo bar
# ruleid: prefer-copy-over-add
ADD https://foo bar
# ruleid: prefer-copy-over-add
ADD foo.tar.gz bar
# ruleid: prefer-copy-over-add
ADD foo.bz2 bar
```
**Correct (Dockerfile - using COPY or ADD for local files):**
```dockerfile
FROM busybox
# ok: prefer-copy-over-add
ADD foo bar
# ok: prefer-copy-over-add
ADD foo* /mydir/
# ok: prefer-copy-over-add
ADD hom?.txt /mydir/o
# ok: prefer-copy-over-add
ADD arr[[]0].txt /mydir/o
# ok: prefer-copy-over-add
ADD --chown=55:mygroup files* /somedir/
# ok: prefer-copy-over-add
ADD --chown=bin files* /somedir/
```
**Incorrect (Dockerfile - using RUN cd):**
Use 'WORKDIR' instead of 'RUN cd ...' for improved clarity and reliability. 'RUN cd ...' may not work as expected in a container.
```dockerfile
FROM busybox
# ruleid: use-workdir
RUN cd semgrep && git clone https://github.com/returntocorp/semgrep
```
**Correct (Dockerfile - using WORKDIR):**
```dockerfile
FROM busybox
# ok: use-workdir
RUN pip3 install semgrep && cd ..
# ok: use-workdir
RUN semgrep -f p/xss
# ok: use-workdir
RUN blah
# ok: use-workdir
RUN blah blahcd
```
**Incorrect (Dockerfile - using apt-get upgrade):**
Packages in base containers should be up-to-date, removing the need to upgrade or dist-upgrade. If a package is out of date, contact the maintainers.
```dockerfile
FROM debian
# ruleid:avoid-apt-get-upgrade
RUN apt-get update && apt-get upgrade
# ruleid:avoid-apt-get-upgrade
RUN apt-get update && apt-get upgrade -y
# ruleid:avoid-apt-get-upgrade
RUN apt-get update && apt-get dist-upgrade
# ruleid:avoid-apt-get-upgrade
RUN apt-get upgrade
```
**Correct (Dockerfile - only updating package lists):**
```dockerfile
FROM debian
# ok: avoid-apt-get-upgrade
RUN apt-get update
```
**Incorrect (Dockerfile - nonsensical commands):**
Some commands do not make sense in a container and should not be used. These include: shutdown, service, ps, free, top, kill, mount, ifconfig, nano, vim.
```dockerfile
FROM busybox
# ruleid: nonsensical-command
RUN top
# ruleid: nonsensical-command
RUN kill 1234
# ruleid: nonsensical-command
RUN ifconfig
# ruleid: nonsensical-command
RUN ps -ef
# ruleid: nonsensical-command
RUN vim /var/log/www/error.log
```
**Correct (Dockerfile - appropriate container commands):**
```dockerfile
FROM busybox
# ok: nonsensical-command
RUN git clone https://github.com/returntocorp/semgrep
# ok: nonsensical-command
RUN pip3 install semgrep
# ok: nonsensical-command
RUN semgrep -f p/xss
```
**Incorrect (Dockerfile - using --platform with FROM):**
Using '--platform' with FROM restricts the image to build on a single platform. Use 'docker buildx --platform=' instead for multi-platform builds.
```dockerfile
# ruleid: avoid-platform-with-from
FROM --platform=x86 busybox
# ruleid: avoid-platform-with-from
FROM --platform=x86 busybox:1.34
# ruleid: avoid-platform-with-from
FROM --platform=x86 busybox AS bb
# ruleid: avoid-platform-with-from
FROM --platform=x86 busybox:1.34 AS bb
```
**Correct (Dockerfile - FROM without platform restriction):**
```dockerfile
# ok: avoid-platform-with-from
FROM busybox
# ok: avoid-platform-with-from
FROM busybox:1.34
# ok: avoid-platform-with-from
FROM busybox AS bb
# ok: avoid-platform-with-from
FROM busybox:1.34 AS bb
```
**Incorrect (Docker Compose - privileged service):**
### Privileged Mode (Docker Compose)
Running containers in privileged mode grants the container the equivalent of root capabilities on the host machine. This can lead to container escapes, privilege escalation, and other security concerns.
**Incorrect:**
```yaml
version: "3.9"
services:
# ok: privileged-service
web:
image: nginx:alpine
worker:
image: my-worker-image:latest
# ruleid:privileged-service
image: my-worker-image:1.0
privileged: true
# ok: privileged-service
db:
image: mysql
```
**Correct (Docker Compose - service without privileged mode):**
**Correct:**
```yaml
version: "3.9"
services:
web:
image: nginx:alpine
worker:
image: my-worker-image:latest
image: my-worker-image:1.0
privileged: false
db:
image: mysql
```
**Incorrect (Docker Compose - writable filesystem):**
Services running with a writable root filesystem may allow malicious applications to download and run additional payloads, or modify container files. Use read-only filesystems when possible.
```yaml
version: "3.9"
services:
# ruleid: writable-filesystem-service
web:
image: nginx:alpine
# ruleid: writable-filesystem-service
worker:
image: my-worker-image:latest
read_only: false
```
**Correct (Docker Compose - read-only filesystem):**
```yaml
version: "3.9"
services:
# ok: writable-filesystem-service
db:
image: mysql
read_only: true
```
**Incorrect (Docker Compose - exposing Docker socket):**
### Exposing Docker Socket
Exposing the host's Docker socket to containers via a volume is equivalent to giving unrestricted root access to your host. Never expose the Docker socket unless absolutely necessary.
**Incorrect:**
```yaml
version: "3.9"
services:
service02:
image: my-worker-image:latest
# ruleid: exposing-docker-socket-volume
worker:
image: my-worker-image:1.0
volumes:
- /tmp/foo:/tmp/foo
- /var/run/docker.sock:/var/run/docker.sock
service05:
image: ubuntu
# ruleid: exposing-docker-socket-volume
volumes:
- /tmp/foo:/tmp/foo
- /run/docker.sock:/run/docker.sock
service14:
image: redis:6
# ruleid: exposing-docker-socket-volume
volumes:
- /var/run/docker.sock
service22:
image: debian:bullseye
# ruleid: exposing-docker-socket-volume
volumes:
- source: /var/run/docker.sock
service23:
image: debian:buster
# ruleid: exposing-docker-socket-volume
volumes:
- source: /var/run/docker.sock
target: /var/run/docker.sock
```
**Correct (Docker Compose - no Docker socket exposure):**
**Correct:**
```yaml
version: "3.9"
services:
service01:
image: nginx:alpine
# ok: exposing-docker-socket-volume
worker:
image: my-worker-image:1.0
volumes:
- /tmp/foo:/tmp/foo
- /tmp/bar:/tmp/bar
service28:
image: mysql:latest
# ok: exposing-docker-socket-volume
volumes:
- source: /tmp/foo
service29:
image: postgres:latest
# ok: exposing-docker-socket-volume
volumes:
- source: /tmp/foo
target: /tmp/bar
- source: /tmp/bar
target: /tmp/foo
- /tmp/data:/tmp/data
```
**Incorrect (Python Docker SDK - arbitrary container run):**
### Arbitrary Container Run (Python Docker SDK)
If unverified user data can reach the `run` or `create` method, it can result in running arbitrary containers.
**Incorrect:**
```python
import docker
client = docker.from_env()
def bad1(user_input):
# ruleid: docker-arbitrary-container-run
def run_container(user_input):
client.containers.run(user_input, 'echo hello world')
def bad2(user_input):
# ruleid: docker-arbitrary-container-run
client.containers.create(user_input, 'echo hello world')
```
**Correct (Python Docker SDK - hardcoded container image):**
**Correct:**
```python
import docker
client = docker.from_env()
def ok1():
# ok: docker-arbitrary-container-run
def run_container():
client.containers.run("alpine", 'echo hello world')
def ok2():
# ok: docker-arbitrary-container-run
client.containers.create("alpine", 'echo hello world')
```
skills/code-security/rules/github-actions.md
+27 -562
View File
@@ -13,298 +13,75 @@ GitHub Actions workflows can be vulnerable to several security issues including
1. **Script Injection**: Using untrusted input (like PR titles or issue bodies) directly in `run:` commands allows attackers to inject arbitrary code
2. **Privileged Triggers**: `pull_request_target` and `workflow_run` events run with elevated privileges, making checkout of untrusted code dangerous
3. **Secrets Exposure**: Improper handling of secrets can leak them in logs or to malicious code
4. **Supply Chain**: Third-party actions not pinned to commit SHAs can be compromised
3. **Supply Chain**: Third-party actions not pinned to commit SHAs can be compromised
---
### Run Shell Injection (CWE-78)
Using variable interpolation `${{...}}` with `github` context data in a `run:` step could allow an attacker to inject their own code into the runner. This would allow them to steal secrets and code. `github` context data can have arbitrary user input and should be treated as untrusted.
Using variable interpolation `${{...}}` with `github` context data in a `run:` step could allow an attacker to inject their own code into the runner. This would allow them to steal secrets and code.
**Incorrect (vulnerable to script injection via PR title):**
```yaml
jobs:
docker-build:
build:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2
- name: Check PR title
# ruleid: run-shell-injection
run: |
title="${{ github.event.pull_request.title }}"
if [[ $title =~ ^octocat ]]; then
echo "PR title starts with 'octocat'"
exit 0
else
echo "PR title did not start with 'octocat'"
exit 1
fi
echo "$title"
```
**Incorrect (vulnerable to injection via workflow inputs):**
```yaml
on:
workflow_dispatch:
inputs:
message_to_print:
type: string
required: false
jobs:
docker-build:
runs-on: ubuntu-latest
steps:
- name: Print a message
# ruleid: run-shell-injection
run: |
echo "${{github.event.inputs.message_to_print}}"
```
**Incorrect (vulnerable to injection via issue title):**
**Correct (use environment variable):**
```yaml
jobs:
docker-build:
build:
runs-on: ubuntu-latest
steps:
- name: Show issue title
# ruleid: run-shell-injection
run: |
echo "${{ github.event.issue.title }}"
```
**Incorrect (vulnerable to injection via commit author email):**
```yaml
jobs:
docker-build:
runs-on: ubuntu-latest
steps:
- name: Show author email
# ruleid: run-shell-injection
run: |
echo "${{ github.event.commits.fix-bug.author.email }}"
```
**Correct (safe use of GitHub context):**
```yaml
jobs:
docker-build:
runs-on: ubuntu-latest
steps:
- name: Push commit hash if PR
if: github.event_name == 'pull_request' && github.event.pull_request.head.repo.full_name == github.repository
# ok: run-shell-injection
run: |
tag=returntocorp/semgrep:${{ github.sha }}
docker build -t "$tag" .
docker push "$tag"
```
**Correct (using secrets safely):**
```yaml
jobs:
docker-build:
runs-on: ubuntu-latest
steps:
- name: benign
# ok: run-shell-injection
run: |
AUTH_HEADER="Authorization: Bearer ${{ secrets.GITHUB_TOKEN }}";
HEADER="Accept: application/vnd.github.v3+json";
```
**Correct (using workflow_run artifacts_url safely):**
```yaml
jobs:
docker-build:
runs-on: ubuntu-latest
steps:
- name: Download and Extract Artifacts
- name: Check PR title
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
# ok: run-shell-injection
PR_TITLE: ${{ github.event.pull_request.title }}
run: |
mkdir -p artifacts && cd artifacts
artifacts_url=${{ github.event.workflow_run.artifacts_url }}
gh api "$artifacts_url" -q '.artifacts[] | [.name, .archive_download_url] | @tsv' | while read artifact
do
IFS=$'\t' read name url <<< "$artifact"
gh api $url > "$name.zip"
unzip -d "$name" "$name.zip"
done
echo "$PR_TITLE"
```
**Fix**: Use an intermediate environment variable with `env:` to store the data and use the environment variable in the `run:` script. Be sure to use double-quotes around the environment variable, like this: `"$ENVVAR"`.
**Fix**: Use an intermediate environment variable with `env:` to store the data and use the environment variable in the `run:` script. Be sure to use double-quotes around the environment variable.
Reference: [GitHub Actions Security Hardening - Script Injections](https://docs.github.com/en/actions/learn-github-actions/security-hardening-for-github-actions#understanding-the-risk-of-script-injections)
---
### GitHub Script Injection (CWE-94)
Using variable interpolation `${{...}}` with `github` context data in `actions/github-script`'s `script:` step could allow an attacker to inject their own code into the runner.
**Incorrect (vulnerable to injection via PR title in github-script):**
```yaml
jobs:
script-run:
runs-on: ubuntu-latest
steps:
- name: Checkout
uses: actions/checkout@v3
- name: Run script 1
uses: actions/github-script@v6
if: steps.report-diff.outputs.passed == 'true'
with:
# ruleid: github-script-injection
script: |
const fs = require('fs');
const body = fs.readFileSync('/tmp/file.txt', {encoding: 'utf8'});
await github.rest.issues.createComment({
issue_number: context.issue.number,
owner: context.repo.owner,
repo: context.repo.repo,
body: '${{ github.event.pull_request.title }}' + body
})
return true;
```
**Incorrect (vulnerable to injection via issue title in github-script):**
```yaml
jobs:
script-run:
runs-on: ubuntu-latest
steps:
- name: Run script 2
uses: actions/github-script@latest
with:
# ruleid: github-script-injection
script: |
const fs = require('fs');
const body = fs.readFileSync('/tmp/${{ github.event.issue.title }}.txt', {encoding: 'utf8'});
await github.rest.issues.createComment({
issue_number: context.issue.number,
owner: context.repo.owner,
repo: context.repo.repo,
body: 'Thanks for reporting!'
})
return true;
```
**Correct (non-github-script action is safe):**
```yaml
jobs:
script-run:
runs-on: ubuntu-latest
steps:
- name: Ok script 1
uses: not-github/custom-action@latest
with:
# ok: github-script-injection
script: |
return ${{ github.event.issue.title }};
```
**Correct (using safe github context like artifacts_url):**
```yaml
jobs:
script-run:
runs-on: ubuntu-latest
steps:
- name: Ok script 2
uses: actions/github-script@latest
with:
# ok: github-script-injection
script: |
console.log('${{ github.event.workflow_run.artifacts_url }}');
await github.rest.issues.createComment({
issue_number: context.issue.number,
owner: context.repo.owner,
repo: context.repo.repo,
body: 'Thanks for reporting!'
})
return true;
```
Reference: [GitHub Actions Untrusted Input](https://securitylab.github.com/research/github-actions-untrusted-input/)
---
### Pull Request Target Code Checkout (CWE-913)
When using `pull_request_target`, the Action runs in the context of the target repository with access to all repository secrets. Checking out the incoming PR code while having access to secrets is dangerous because you may inadvertently execute arbitrary code from the incoming PR.
**Incorrect (checking out PR code with pull_request_target):**
```yaml
# cf. https://securitylab.github.com/research/github-actions-preventing-pwn-requests/
# INSECURE. Provided as an example only.
on:
pull_request_target:
pull_request:
jobs:
build:
name: Build and test
runs-on: ubuntu-latest
steps:
# ruleid: pull-request-target-code-checkout
- uses: actions/checkout@v2
with:
ref: ${{ github.event.pull_request.head.sha }}
- uses: actions/setup-node@v1
- run: |
npm install
npm build
- uses: completely/fakeaction@v2
with:
arg1: ${{ secrets.supersecret }}
```
**Incorrect (using merge ref with pull_request_target):**
```yaml
on:
pull_request_target:
pull_request:
jobs:
# cf. https://github.com/justinsteven/advisories/blob/master/2021_github_actions_checkspelling_token_leak_via_advice_symlink.md
spelling:
name: Spell checking
runs-on: ubuntu-latest
steps:
# ruleid: pull-request-target-code-checkout
- name: checkout-merge
if: contains(github.event_name, 'pull_request')
uses: actions/checkout@v2
with:
ref: refs/pull/${{github.event.pull_request.number}}/merge
- run: npm install && npm build
```
**Correct (no checkout of PR code):**
```yaml
on:
pull_request_target:
pull_request:
jobs:
this-is-safe-because-no-checkout:
name: Echo
safe-job:
runs-on: ubuntu-latest
steps:
# ok: pull-request-target-code-checkout
- name: echo
run: |
echo "Hello, world"
run: echo "Hello, world"
```
Reference: [GitHub Actions Preventing Pwn Requests](https://securitylab.github.com/research/github-actions-preventing-pwn-requests/)
@@ -319,382 +96,70 @@ Similar to `pull_request_target`, when using `workflow_run`, the Action runs in
```yaml
on:
workflow_run:
workflows: ["smth-else"]
types:
- completed
pull_request:
workflows: ["CI"]
types: [completed]
jobs:
build:
name: Build and test
runs-on: ubuntu-latest
steps:
# ruleid: workflow-run-target-code-checkout
- uses: actions/checkout@v2
with:
ref: ${{ github.event.workflow_run.head.sha }}
- uses: actions/setup-node@v1
- run: |
npm install
npm build
- uses: completely/fakeaction@v2
with:
arg1: ${{ secrets.supersecret }}
```
**Incorrect (using merge ref with workflow_run):**
```yaml
on:
workflow_run:
workflows: ["smth-else"]
types:
- completed
pull_request:
jobs:
spelling:
name: Spell checking
runs-on: ubuntu-latest
steps:
# ruleid: workflow-run-target-code-checkout
- name: checkout-merge
if: contains(github.event_name, 'pull_request')
uses: actions/checkout@v2
with:
ref: refs/pull/${{github.event.workflow_run.number}}/merge
- run: npm install
```
**Correct (no checkout of PR code):**
```yaml
on:
workflow_run:
workflows: ["smth-else"]
types:
- completed
pull_request:
workflows: ["CI"]
types: [completed]
jobs:
this-is-safe-because-no-checkout:
name: Echo
safe-job:
runs-on: ubuntu-latest
steps:
# ok: workflow-run-target-code-checkout
- name: echo
run: |
echo "Hello, world"
- run: echo "Safe operation"
```
Reference: [GitHub Privilege Escalation Vulnerability](https://www.legitsecurity.com/blog/github-privilege-escalation-vulnerability)
---
### Curl Eval (CWE-78)
### Third-Party Action Not Pinned to Commit SHA (CWE-1357)
Data is being eval'd from a `curl` command. An attacker with control of the server in the `curl` command could inject malicious code into the `eval`, resulting in a system compromise.
An action sourced from a third-party repository on GitHub is not pinned to a full length commit SHA. Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release.
**Incorrect (eval'ing data from curl):**
**Incorrect (using tag reference):**
```yaml
name: Build and deploy Semgrep scanner lambda
on:
workflow_dispatch:
push:
branches: develop
jobs:
docker-build:
runs-on: ubuntu-latest
env:
workdir: lambdas/run-semgrep
steps:
- uses: actions/checkout@v2
- name:
blah
# ruleid: curl-eval
run: |
CONTENTS=$(curl https://blah.com)
eval $CONTENTS
```
**Correct (safe docker build without eval):**
```yaml
name: Build and deploy Semgrep scanner lambda
on:
workflow_dispatch:
push:
branches: develop
jobs:
docker-build:
runs-on: ubuntu-latest
env:
workdir: lambdas/run-semgrep
steps:
- uses: actions/checkout@v2
- name: Build Docker image
working-directory:
${{ env.workdir }}/src
# ok: curl-eval
run: docker build -t semgrep-scanner:latest .
```
**Fix**: Avoid eval'ing untrusted data if you can. If you must do this, consider checking the SHA sum of the content returned by the server to verify its integrity.
Reference: [GitHub Actions Security Hardening - Script Injections](https://docs.github.com/en/actions/learn-github-actions/security-hardening-for-github-actions#understanding-the-risk-of-script-injections)
---
### Allowed Unsecure Commands (CWE-749)
The environment variable `ACTIONS_ALLOW_UNSECURE_COMMANDS` grants permissions to use the deprecated `set-env` and `add-path` commands, which have a vulnerability that could allow environment variable modification by attackers.
**Incorrect (enabling unsecure commands in step env):**
```yaml
on: pull_request
name: command-processing-test
jobs:
dangerous-job:
name: example
runs-on: ubuntu-latest
steps:
- name: dont-do-this
env:
# ruleid: allowed-unsecure-commands
ACTIONS_ALLOW_UNSECURE_COMMANDS: true
run: |
echo "don't do this"
```
**Incorrect (enabling unsecure commands in job env):**
```yaml
on: pull_request
name: command-processing-test
jobs:
another-dangerous-job:
name: example2
runs-on: ubuntu-latest
env:
# ruleid: allowed-unsecure-commands
ACTIONS_ALLOW_UNSECURE_COMMANDS: true
steps:
- name: or-this
run: |
echo "seriously, dont"
```
**Correct (no unsecure commands):**
```yaml
on: pull_request
name: command-processing-test
jobs:
this-is-ok:
name: example3
runs-on: ubuntu-latest
env: PREFIX = "~~^_^~~"
run: |
echo "$PREFIX hello"
```
**Fix**: Don't use `ACTIONS_ALLOW_UNSECURE_COMMANDS`. Instead, use Environment Files.
Reference: [GitHub Actions Environment Files](https://github.com/actions/toolkit/blob/main/docs/commands.md#environment-files)
---
### Third-Party Action Not Pinned to Commit SHA (CWE-1357, CWE-353)
An action sourced from a third-party repository on GitHub is not pinned to a full length commit SHA. Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. This helps mitigate the risk of a bad actor adding a backdoor to the action's repository.
**Incorrect (using tag or branch reference):**
```yaml
on:
pull_request_target:
pull_request:
jobs:
build:
name: Build and test
runs-on: ubuntu-latest
steps:
# ruleid: third-party-action-not-pinned-to-commit-sha
- uses: fakerepo/comment-on-pr@v1
with:
message: |
Thank you!
# ruleid: third-party-action-not-pinned-to-commit-sha
- uses: fakerepo/comment-on-pr
with:
message: |
Thank you!
```
**Incorrect (using short SHA):**
```yaml
jobs:
build:
name: Build and test
runs-on: ubuntu-latest
steps:
# ruleid: third-party-action-not-pinned-to-commit-sha
- uses: completely/fakeaction@5fd3084
with:
arg2: ${{ secrets.supersecret2 }}
```
**Incorrect (unpinned Docker action):**
```yaml
jobs:
build:
name: Build and test
runs-on: ubuntu-latest
steps:
# ruleid: third-party-action-not-pinned-to-commit-sha
- uses: docker://gcr.io/cloud-builders/gradle
# ruleid: third-party-action-not-pinned-to-commit-sha
- uses: docker://alpine:3.8
message: "Thank you!"
```
**Correct (pinned to full commit SHA):**
```yaml
jobs:
build:
name: Build and test
runs-on: ubuntu-latest
steps:
# ok: third-party-action-not-pinned-to-commit-sha
- uses: completely/fakeaction@5fd3084fc36e372ff1fff382a39b10d03659f355
- uses: fakerepo/comment-on-pr@5fd3084fc36e372ff1fff382a39b10d03659f355
with:
arg2: ${{ secrets.supersecret2 }}
message: "Thank you!"
```
**Correct (Docker action with pinned digest):**
```yaml
jobs:
build:
name: Build and test
runs-on: ubuntu-latest
steps:
# ok: third-party-action-not-pinned-to-commit-sha
- uses: docker://alpine@sha256:402d21757a03a114d273bbe372fa4b9eca567e8b6c332fa7ebf982b902207242
```
**Correct (GitHub-owned actions don't need pinning):**
```yaml
jobs:
build:
name: Build and test
runs-on: ubuntu-latest
steps:
# ok: third-party-action-not-pinned-to-commit-sha
- uses: actions/checkout@v2
with:
ref: ${{ github.event.pull_request.head.sha }}
# ok: third-party-action-not-pinned-to-commit-sha
- uses: actions/setup-node@master
# ok: third-party-action-not-pinned-to-commit-sha
- name: Upload SARIF file for GitHub Advanced Security Dashboard
uses: github/codeql-action/upload-sarif@v2
with:
sarif_file: semgrep.sarif
if: always()
```
**Correct (local actions don't need pinning):**
```yaml
jobs:
build:
name: Build and test
runs-on: ubuntu-latest
steps:
# ok: third-party-action-not-pinned-to-commit-sha
- uses: ./.github/actions/do-a-local-action
with:
arg1: ${{ secrets.supersecret1 }}
build2:
name: Build and test using a local workflow
# ok: third-party-action-not-pinned-to-commit-sha
uses: ./.github/workflows/use_a_local_workflow.yml@master
secrets: inherit
with:
examplearg: true
```
Note: GitHub-owned actions (`actions/*`, `github/*`) and local actions (`./.github/actions/*`) don't require SHA pinning.
Reference: [GitHub Actions Security Hardening - Using Third-Party Actions](https://docs.github.com/en/actions/security-guides/security-hardening-for-github-actions#using-third-party-actions)
---
### Unsafe Add-Mask Workflow Command (CWE-200)
GitHub Actions provides the `add-mask` workflow command to mask sensitive data in workflow logs. However, if workflow commands have been stopped (via `echo "::stop-commands::$stopMarker"`), sensitive data can be leaked. An attacker could copy the workflow to another branch and add a payload to stop workflow command processing, exposing secrets.
**Incorrect (using add-mask which can be bypassed):**
```yaml
name: Test Workflow
on:
push:
branches:
- main
jobs:
test-job:
runs-on: ubuntu-latest
steps:
- name: Checkout code
uses: actions/checkout@v2
- name: Setup Python
uses: actions/setup-python@v2
with:
python-version: '3.8'
- name: Run script to generate token
run: |
TOKEN=$(openssl rand -hex 16)
# ruleid: unsafe-add-mask-workflow-command
echo "::add-mask::$TOKEN"
echo "TOKEN=$TOKEN" >> $GITHUB_ENV
- name: Use the token
run: |
echo "Using the token in the next step"
curl -H "Authorization: Bearer $TOKEN" https://api.example.com
- name: Print GitHub context
run: |
echo "GitHub context:"
echo "${{ toJSON(github) }}"
# ruleid: unsafe-add-mask-workflow-command
echo "::add-mask::${{ secrets.GITHUB_TOKEN }}"
```
**Fix**: Prefer using GitHub's native secrets handling rather than relying on `add-mask` for security-critical masking. Consider the risk that an attacker with write access could modify the workflow to bypass masking.
Reference: [GitHub Actions Workflow Commands - Masking](https://github.com/github/docs/blob/main/content/actions/using-workflows/workflow-commands-for-github-actions.md#masking-a-value-in-a-log)
---
**References:**
- CWE-78: Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection')
- CWE-94: Improper Control of Generation of Code ('Code Injection')
- CWE-200: Exposure of Sensitive Information to an Unauthorized Actor
- CWE-749: Exposed Dangerous Method or Function
- CWE-913: Improper Control of Dynamically-Managed Code Resources
- CWE-1357: Reliance on Insufficiently Trustworthy Component
- CWE-353: Missing Support for Integrity Check
- [GitHub Actions Security Hardening](https://docs.github.com/en/actions/learn-github-actions/security-hardening-for-github-actions)
- [GitHub Security Lab - Preventing Pwn Requests](https://securitylab.github.com/research/github-actions-preventing-pwn-requests/)
- [GitHub Security Lab - Untrusted Input](https://securitylab.github.com/research/github-actions-untrusted-input/)
- [OWASP Top 10 A03:2021 - Injection](https://owasp.org/Top10/A03_2021-Injection/)
skills/code-security/rules/insecure-crypto.md
+96 -804
View File
@@ -5,898 +5,190 @@ impact: HIGH
## Avoid Insecure Cryptography
Using weak or broken cryptographic algorithms puts sensitive data at risk. Attackers can exploit known vulnerabilities in deprecated algorithms to decrypt data, forge signatures, or predict "random" values. Weak algorithms include MD5 and SHA1 for hashing (collision attacks are practical), DES/RC4/Blowfish for encryption (deprecated due to small key or block sizes), RSA keys below 2048 bits, ECB mode (reveals patterns), and non-cryptographic random number generators. CWE-327: Use of a Broken or Risky Cryptographic Algorithm. CWE-328: Use of Weak Hash. CWE-326: Inadequate Encryption Strength.
Using weak or broken cryptographic algorithms puts sensitive data at risk. Attackers can exploit known vulnerabilities in deprecated algorithms to decrypt data, forge signatures, or predict "random" values.
**Incorrect (Python - MD5 hashing):**
**Key vulnerabilities:**
- **Weak hashing:** MD5 and SHA1 are vulnerable to collision attacks
- **Weak encryption:** DES is deprecated due to small key/block sizes
**References:** CWE-327 (Broken Crypto Algorithm), CWE-328 (Weak Hash), CWE-326 (Inadequate Encryption Strength)
---
### Python
**Incorrect (MD5/SHA1 hashing):**
```python
import hashlib
# Using MD5 for hashing
hashlib.md5(1)
hashlib.md5(1).hexdigest()
abc = str.replace(hashlib.md5("1"), "###")
print(hashlib.md5("1"))
foo = hashlib.md5(data, usedforsecurity=True)
hash_val = hashlib.md5(data).hexdigest()
hash_val = hashlib.sha1(data).hexdigest()
```
**Incorrect (Python - SHA1 hashing):**
**Correct (SHA256 hashing):**
```python
import hashlib
# Using SHA1 for hashing
hashlib.sha1(1)
hash_val = hashlib.sha256(data).hexdigest()
```
**Incorrect (Python - SHA1 with cryptography library):**
**Incorrect (DES cipher):**
```python
from cryptography.hazmat.primitives import hashes
hashes.SHA1()
```
**Correct (Python - SHA256 hashing):**
```python
import hashlib
# Using secure hash algorithm
hashlib.sha256(1)
# With cryptography library
from cryptography.hazmat.primitives import hashes
hashes.SHA256()
hashes.SHA3_256()
```
**Incorrect (Python - DES cipher):**
```python
from Crypto.Cipher import DES as pycrypto_des
from Cryptodome.Cipher import DES as pycryptodomex_des
from Crypto.Cipher import DES
key = b'-8B key-'
plaintext = b'We are no longer the knights who say ni!'
nonce = Random.new().read(pycrypto_des.block_size/2)
ctr = Counter.new(pycrypto_des.block_size*8/2, prefix=nonce)
cipher = pycrypto_des.new(key, pycrypto_des.MODE_CTR, counter=ctr)
cipher = pycryptodomex_des.new(key, pycryptodomex_des.MODE_CTR, counter=ctr)
cipher = DES.new(key, DES.MODE_CTR, counter=ctr)
```
**Incorrect (Python - RC4/ARC4 cipher):**
```python
from Crypto.Cipher import ARC4 as pycrypto_arc4
from Cryptodome.Cipher import ARC4 as pycryptodomex_arc4
from cryptography.hazmat.primitives.ciphers import algorithms
key = b'Very long and confidential key'
tempkey = SHA.new(key+nonce).digest()
cipher = pycrypto_arc4.new(tempkey)
cipher = pycryptodomex_arc4.new(tempkey)
# With cryptography library
cipher = Cipher(algorithms.ARC4(key), mode=None, backend=default_backend())
```
**Correct (Python - AES cipher):**
**Correct (AES cipher):**
```python
from Crypto.Cipher import AES
from cryptography.hazmat.primitives.ciphers import Cipher, algorithms, modes
key = b'Sixteen byte key'
cipher = AES.new(key, AES.MODE_EAX, nonce=nonce)
# With cryptography library
cipher = Cipher(algorithms.AES(key), modes.CBC(iv), backend=default_backend())
```
**Incorrect (Python - ECB mode):**
---
```python
from cryptography.hazmat.primitives.ciphers.modes import ECB
### JavaScript
mode = ECB(iv)
```
**Incorrect (MD5 hashing):**
**Correct (Python - CBC mode):**
```javascript
const crypto = require("crypto");
```python
from cryptography.hazmat.primitives.ciphers.modes import CBC
mode = CBC(iv)
```
**Incorrect (Python - weak RSA key size):**
```python
from cryptography.hazmat.primitives.asymmetric import rsa
rsa.generate_private_key(public_exponent=65537,
key_size=1024,
backend=backends.default_backend())
```
**Correct (Python - strong RSA key size):**
```python
from cryptography.hazmat.primitives.asymmetric import rsa
rsa.generate_private_key(public_exponent=65537,
key_size=2048,
backend=backends.default_backend())
```
**Incorrect (Python - JWT none algorithm):**
```python
import jwt
encoded = jwt.encode({'some': 'payload'}, None, algorithm='none')
jwt.decode(encoded, None, algorithms=['none'])
```
**Correct (Python - JWT with proper algorithm):**
```python
import jwt
encoded = jwt.encode({'some': 'payload'}, secret_key, algorithm='HS256')
```
**Incorrect (Java - MD5 hashing):**
```java
import java.security.MessageDigest;
public byte[] bad1(String password) {
MessageDigest md5Digest = MessageDigest.getInstance("MD5");
md5Digest.update(password.getBytes());
byte[] hashValue = md5Digest.digest();
return hashValue;
function hashPassword(pwtext) {
return crypto.createHash("md5").update(pwtext).digest("hex");
}
java.security.MessageDigest md = java.security.MessageDigest.getInstance("MD5");
```
**Incorrect (Java - SHA1 hashing):**
**Correct (SHA256 hashing):**
```java
import java.security.MessageDigest;
import org.apache.commons.codec.digest.DigestUtils;
```javascript
const crypto = require("crypto");
MessageDigest sha1Digest = MessageDigest.getInstance("SHA-1");
sha1Digest.update(password.getBytes());
byte[] hashValue = DigestUtils.getSha1Digest().digest(password.getBytes());
java.security.MessageDigest md = java.security.MessageDigest.getInstance("SHA1", "SUN");
function hashPassword(pwtext) {
return crypto.createHash("sha256").update(pwtext).digest("hex");
}
```
**Correct (Java - SHA-512 hashing):**
---
### Java
**Incorrect (MD5/SHA1 hashing):**
```java
import java.security.MessageDigest;
MessageDigest sha512Digest = MessageDigest.getInstance("SHA-512");
sha512Digest.update(password.getBytes());
byte[] hashValue = sha512Digest.digest();
MessageDigest md5 = MessageDigest.getInstance("MD5");
md5.update(password.getBytes());
byte[] hash = md5.digest();
MessageDigest sha1 = MessageDigest.getInstance("SHA-1");
```
**Incorrect (Java - DES cipher):**
**Correct (SHA-512 hashing):**
```java
import java.security.MessageDigest;
MessageDigest sha512 = MessageDigest.getInstance("SHA-512");
sha512.update(password.getBytes());
byte[] hash = sha512.digest();
```
**Incorrect (DES cipher):**
```java
Cipher c = Cipher.getInstance("DES/ECB/PKCS5Padding");
c.init(Cipher.ENCRYPT_MODE, k, iv);
Cipher c = Cipher.getInstance("DES");
```
**Incorrect (Java - RC4 cipher):**
```java
Cipher.getInstance("RC4");
useCipher(Cipher.getInstance("RC4"));
```
**Correct (Java - AES with GCM):**
**Correct (AES with GCM):**
```java
Cipher c = Cipher.getInstance("AES/GCM/NoPadding");
c.init(Cipher.ENCRYPT_MODE, k, iv);
Cipher.getInstance("AES/CBC/PKCS7PADDING");
```
**Incorrect (Java - ECB mode):**
---
```java
Cipher c = Cipher.getInstance("AES/ECB/NoPadding");
c.init(Cipher.ENCRYPT_MODE, k, iv);
byte[] cipherText = c.doFinal(plainText);
```
### Go
**Correct (Java - GCM mode):**
```java
Cipher c = Cipher.getInstance("AES/GCM/NoPadding");
c.init(Cipher.ENCRYPT_MODE, k, iv);
byte[] cipherText = c.doFinal(plainText);
```
**Incorrect (Java - weak RSA key size):**
```java
import java.security.KeyPairGenerator;
KeyPairGenerator keyGen = KeyPairGenerator.getInstance("RSA");
keyGen.initialize(512);
```
**Correct (Java - strong RSA key size):**
```java
import java.security.KeyPairGenerator;
KeyPairGenerator keyGen = KeyPairGenerator.getInstance("RSA");
keyGen.initialize(2048);
```
**Incorrect (Java - weak random number generator):**
```java
float rand = new java.util.Random().nextFloat();
new java.util.Random().nextInt();
double value = java.lang.Math.random();
```
**Correct (Java - secure random):**
```java
double value2 = java.security.SecureRandom();
```
**Incorrect (Java - weak SSL context):**
```java
SSLContext ctx = SSLContext.getInstance("SSL");
SSLContext ctx = SSLContext.getInstance("TLS");
SSLContext ctx = SSLContext.getInstance("TLSv1");
SSLContext ctx = SSLContext.getInstance("SSLv3");
SSLContext ctx = SSLContext.getInstance("TLSv1.1");
```
**Correct (Java - secure TLS version):**
```java
SSLContext ctx = SSLContext.getInstance("TLSv1.2");
SSLContext ctx = SSLContext.getInstance("TLSv1.3");
```
**Incorrect (JavaScript - weak pseudo-random bytes):**
```javascript
// Using pseudoRandomBytes which is not cryptographically secure
crypto.pseudoRandomBytes
```
**Correct (JavaScript - secure random bytes):**
```javascript
// Using cryptographically secure random bytes
crypto.randomBytes
```
**Incorrect (JavaScript - MD5 for password hashing):**
```javascript
const crypto = require("crypto");
function ex1(user, pwtext) {
digest = crypto.createHash("md5").update(pwtext).digest("hex");
user.setPassword(digest);
}
```
**Correct (JavaScript - SHA256 for password hashing):**
```javascript
const crypto = require("crypto");
function ok1(user, pwtext) {
digest = crypto.createHash("sha256").update(pwtext).digest("hex");
user.setPassword(digest);
}
```
**Incorrect (JavaScript - JWT none algorithm):**
```javascript
const jose = require("jose");
const { JWK, JWT } = jose;
const token = JWT.verify('token-here', JWK.None);
```
**Correct (JavaScript - JWT with proper key):**
```javascript
const jose = require("jose");
const { JWK, JWT } = jose;
const token = JWT.verify('token-here', secretKey);
```
**Incorrect (Go - MD5 hashing):**
**Incorrect (MD5 hashing):**
```go
import (
"crypto/md5"
"fmt"
"io"
)
func test_md5() {
func hashData(data []byte) {
h := md5.New()
if _, err := io.Copy(h, f); err != nil {
log.Fatal(err)
}
fmt.Printf("%x", md5.Sum(nil))
h.Write(data)
fmt.Printf("%x", h.Sum(nil))
}
```
**Incorrect (Go - SHA1 hashing):**
**Correct (SHA256 hashing):**
```go
import (
"crypto/sha1"
"crypto/sha256"
"fmt"
"io"
)
func test_sha1() {
h := sha1.New()
if _, err := io.Copy(h, f); err != nil {
log.Fatal(err)
}
fmt.Printf("%x", sha1.Sum(nil))
func hashData(data []byte) {
h := sha256.New()
h.Write(data)
fmt.Printf("%x", h.Sum(nil))
}
```
**Incorrect (Go - DES cipher):**
**Incorrect (DES cipher):**
```go
import "crypto/des"
func test_des() {
ede2Key := []byte("example key 1234")
var tripleDESKey []byte
tripleDESKey = append(tripleDESKey, ede2Key[:16]...)
tripleDESKey = append(tripleDESKey, ede2Key[:8]...)
_, err := des.NewTripleDESCipher(tripleDESKey)
func encrypt() {
key := []byte("example key 1234")
block, _ := des.NewCipher(key[:8])
}
```
**Incorrect (Go - RC4 cipher):**
**Correct (AES cipher):**
```go
import "crypto/rc4"
import "crypto/aes"
func test_rc4() {
key := []byte{1, 2, 3, 4, 5, 6, 7}
c, err := rc4.NewCipher(key)
dst := make([]byte, len(src))
c.XORKeyStream(dst, src)
func encrypt() {
key := []byte("example key 12345678901234567890")
block, _ := aes.NewCipher(key[:32])
}
```
**Incorrect (Go - weak RSA key size):**
```go
import (
"crypto/rand"
"crypto/rsa"
)
pvk, err := rsa.GenerateKey(rand.Reader, 1024)
```
**Correct (Go - strong RSA key size):**
```go
import (
"crypto/rand"
"crypto/rsa"
)
pvk, err := rsa.GenerateKey(rand.Reader, 2048)
```
**Incorrect (Go - weak random number generator):**
```go
import mrand "math/rand"
import mrand "math/rand/v2"
```
**Correct (Go - secure random):**
```go
import "crypto/rand"
good, _ := rand.Read(nil)
```
**Incorrect (Ruby - MD5 hashing):**
```ruby
require 'digest'
md5 = Digest::MD5.hexdigest 'abc'
md5 = Digest::MD5.new
md5 = Digest::MD5.base64digest 'abc'
md5 = Digest::MD5.digest 'abc'
digest = OpenSSL::Digest::MD5.new
digest = OpenSSL::Digest::MD5.hexdigest 'abc'
digest = OpenSSL::Digest::MD5.base64digest 'abc'
digest = OpenSSL::Digest::MD5.digest 'abc'
```
**Incorrect (Ruby - SHA1 hashing):**
```ruby
require 'digest'
sha = Digest::SHA1.hexdigest 'abc'
sha = Digest::SHA1.new
sha = Digest::SHA1.base64digest 'abc'
sha = Digest::SHA1.digest 'abc'
digest = OpenSSL::Digest::SHA1.new
digest = OpenSSL::Digest::SHA1.hexdigest 'abc'
OpenSSL::HMAC.hexdigest("sha1", key, data)
```
**Correct (Ruby - SHA256 hashing):**
```ruby
require 'digest'
digest = OpenSSL::Digest::SHA256.new
digest = OpenSSL::Digest::SHA256.hexdigest 'abc'
OpenSSL::HMAC.hexdigest("SHA256", key, data)
user.set_password Digest::SHA256.hexdigest pwtext
```
**Incorrect (Ruby - MD5 for password hashing):**
```ruby
require 'digest'
def ex1 (user, pwtext)
user.set_password Digest::MD5.hexdigest pwtext
end
def ex2 (user, pwtext)
md5 = Digest::MD5.new
md5.update pwtext
md5 << salt(pwtext)
dig = md5.hexdigest
user.set_password dig
end
```
**Correct (Ruby - SHA256 for password hashing):**
```ruby
require 'digest'
def ok1 (user, pwtext)
user.set_password Digest::SHA256.hexdigest pwtext
end
def ok2 (user, pwtext)
sha = Digest::SHA256.new
sha.update pwtext
sha << salt(pwtext)
dig = sha.hexdigest
user.set_password dig
end
```
**Incorrect (Ruby - weak RSA key size):**
```ruby
class Test
$key = 512
@key2 = 512
OpenSSL::PKey::RSA.new(@key2)
OpenSSL::PKey::RSA.new 512
key = OpenSSL::PKey::RSA.new($key)
end
```
**Correct (Ruby - strong RSA key size):**
```ruby
class Test
$pass1 = 2048
@pass2 = 2048
key = OpenSSL::PKey::RSA.new($pass1)
key = OpenSSL::PKey::RSA.new(@pass2)
key = OpenSSL::PKey::RSA.new(2048)
end
```
**Incorrect (Kotlin - MD5 hashing):**
```kotlin
import java.security.MessageDigest
import org.apache.commons.codec.digest.DigestUtils
public fun md5(password: String): ByteArray {
val md5Digest: MessageDigest = MessageDigest.getInstance("MD5")
md5Digest.update(password.getBytes())
val hashValue: ByteArray = md5Digest.digest()
return hashValue
}
public fun md5_digestutil(password: String): ByteArray {
val hashValue: ByteArray = DigestUtils.getMd5Digest().digest(password.getBytes())
return hashValue
}
```
**Incorrect (Kotlin - SHA1 hashing):**
```kotlin
import java.security.MessageDigest
import org.apache.commons.codec.digest.DigestUtils
var sha1Digest: MessageDigest = MessageDigest.getInstance("SHA1")
var sha1Digest: MessageDigest = MessageDigest.getInstance("SHA-1")
val hashValue: Array<Byte> = DigestUtils.getSha1Digest().digest(password.getBytes())
```
**Correct (Kotlin - SHA256 hashing):**
```kotlin
import java.security.MessageDigest
val sha256Digest: MessageDigest = MessageDigest.getInstance("SHA256")
sha256Digest.update(password.getBytes())
val hashValue: ByteArray = sha256Digest.digest()
```
**Incorrect (Kotlin - ECB mode):**
```kotlin
class ECBCipher {
public fun ecbCipher(): Void {
val c: Cipher = Cipher.getInstance("AES/ECB/NoPadding")
c.init(Cipher.ENCRYPT_MODE, k, iv)
val cipherText = c.doFinal(plainText)
}
public fun ecbCipher2(): Void {
var c = Cipher.getInstance("AES/ECB/NoPadding")
c.init(Cipher.ENCRYPT_MODE, k, iv)
val cipherText = c.doFinal(plainText)
}
}
```
**Correct (Kotlin - GCM mode):**
```kotlin
class ECBCipher {
public fun noEcbCipher(): Void {
var c = Cipher.getInstance("AES/GCM/NoPadding")
c.init(Cipher.ENCRYPT_MODE, k, iv)
val cipherText = c.doFinal(plainText)
}
}
```
**Incorrect (Kotlin - weak RSA key size):**
```kotlin
import java.security.KeyPairGenerator
fun rsaWeak(): Void {
val keyGen: KeyPairGenerator = KeyPairGenerator.getInstance("RSA")
keyGen.initialize(512)
}
```
**Correct (Kotlin - strong RSA key size):**
```kotlin
import java.security.KeyPairGenerator
fun rsaOK(): Void {
val keyGen = KeyPairGenerator.getInstance("RSA");
keyGen.initialize(2048);
}
```
**Incorrect (C# - DES or RC2 cipher):**
```csharp
using System.Security.Cryptography;
public void CreateDES1() {
var key = DES.Create();
}
public void CreateDES2() {
var key = DES.Create("ImplementationName");
}
public void CreateRC21() {
var key = RC2.Create();
}
public void CreateRC22() {
var key = RC2.Create("ImplementationName");
}
```
**Correct (C# - AES cipher):**
```csharp
using System.Security.Cryptography;
public void CreateAes1() {
var key = Aes.Create();
}
public void CreateAes2() {
var key = Aes.Create("ImplementationName");
}
```
**Incorrect (C# - ECB mode):**
```csharp
using System.Security.Cryptography;
public void EncryptWithAesEcb() {
Aes key = Aes.Create();
key.Mode = CipherMode.ECB;
using var encryptor = key.CreateEncryptor();
byte[] msg = new byte[32];
var cipherText = encryptor.TransformFinalBlock(msg, 0, msg.Length);
}
public void EncryptWithAesEcb2() {
Aes key = Aes.Create();
byte[] msg = new byte[32];
var cipherText = key.EncryptEcb(msg, PaddingMode.PKCS7);
}
```
**Correct (C# - CBC mode):**
```csharp
using System.Security.Cryptography;
public void EncryptWithAesCbc() {
Aes key = Aes.Create();
key.Mode = CipherMode.CBC;
using var encryptor = key.CreateEncryptor();
byte[] msg = new byte[32];
var cipherText = encryptor.TransformFinalBlock(msg, 0, msg.Length);
}
public void EncryptWithAesCbc2() {
Aes key = Aes.Create();
byte[] msg = new byte[32];
byte[] iv = new byte[16];
var cipherText = key.EncryptCbc(msg, iv, PaddingMode.PKCS7);
}
```
**Incorrect (C# - weak RNG for key generation):**
```csharp
using System.Security.Cryptography;
public void GenerateBadKey() {
var rng = new System.Random();
byte[] key = new byte[16];
rng.NextBytes(key);
SymmetricAlgorithm cipher = Aes.Create();
cipher.Key = key;
}
public void GenerateBadKeyGcm() {
var rng = new System.Random();
byte[] key = new byte[16];
rng.NextBytes(key);
var cipher = new AesGcm(key);
}
```
**Correct (C# - secure RNG for key generation):**
```csharp
using System.Security.Cryptography;
public void GenerateGoodKey() {
var rng = System.Security.Cryptography.RandomNumberGenerator.Create();
byte[] key = new byte[16];
rng.GetBytes(key);
var cipher = Aes.Create();
cipher.Key = key;
}
public void GenerateGoodKeyGcm() {
var rng = System.Security.Cryptography.RandomNumberGenerator.Create();
byte[] key = new byte[16];
rng.GetBytes(key);
var cipher = new AesGcm(key);
}
```
**Incorrect (PHP - weak crypto functions):**
```php
<?php
$hashed_password = crypt('mypassword');
$hashed_password = md5('mypassword');
$hashed_password = md5_file('filename.txt');
$hashed_password = sha1('mypassword');
$hashed_password = sha1_file('filename.txt');
$hashed_password = str_rot13('totally secure');
```
**Correct (PHP - secure hashing):**
```php
<?php
$hashed_password = sodium_crypto_generichash('mypassword');
```
**Incorrect (PHP - MD5 for password hashing):**
```php
<?php
function test1($value) {
$pass = md5($value);
$user->setPassword($pass);
}
function test2($value) {
$pass = hash('md5', $value);
$user->setPassword($pass);
}
```
**Correct (PHP - SHA256 for password hashing):**
```php
<?php
function okTest1($value) {
$pass = hash('sha256', $value);
$user->setPassword($pass);
}
```
**Incorrect (Swift - insecure random number generators):**
```swift
import Foundation
func example() -> Void {
let randomInt = Int.random(in: 0..<6)
let randomDouble = Double.random(in: 2.71828...3.14159)
let randomBool = Bool.random()
let diceRoll = Int(arc4random_uniform(6) + 1)
let a = Int.random(in: 0 ... 10)
var k: Int = random() % 10;
let randomNumber = arc4random()
arc4random_buf(&r, MemoryLayout<Self>.size)
let x = Int.random(in: 1...100)
var g = SystemRandomNumberGenerator()
let y = Int.random(in: 1...100, using: &g)
}
```
**Correct (Swift - SecCopyRandomBytes):**
```swift
import Security
var randomBytes = [UInt8](repeating: 0, count: 16)
let status = SecRandomCopyBytes(kSecRandomDefault, randomBytes.count, &randomBytes)
```
**Incorrect (Scala - insecure random number generator):**
```scala
class Test {
def bad1() {
import scala.util.Random
val result = Seq.fill(16)(Random.nextInt)
return result.map("%02x" format _).mkString
}
}
```
**Correct (Scala - SecureRandom):**
```scala
class Test {
def ok1() {
import java.security.SecureRandom
val rand = new SecureRandom()
val value = Array.ofDim[Byte](16)
rand.nextBytes(value)
return value.map("%02x" format _).mkString
}
}
```
**Incorrect (Scala - RSA without OAEP padding):**
```scala
class RSACipher {
def badRSACipher(): Void =
try {
val c = Cipher.getInstance("RSA/None/NoPadding")
c.init(Cipher.ENCRYPT_MODE, k, iv)
val cipherText = c.doFinal(plainText)
} catch {
case NonFatal(e) => throw new RuntimeException("Encrypt error", e)
}
}
```
**Correct (Scala - RSA with OAEP padding):**
```scala
class RSACipher {
def okRSACipher(): Void =
try {
var c = Cipher.getInstance("RSA/ECB/OAEPWithMD5AndMGF1Padding")
c.init(Cipher.ENCRYPT_MODE, k, iv)
val cipherText = c.doFinal(plainText)
} catch {
case NonFatal(e) => throw new RuntimeException("Encrypt error", e)
}
}
```
**Incorrect (Rust - insecure hash algorithms):**
```rust
use md2::{Md2};
use md4::{Md4};
use md5::{Md5};
use sha1::{Sha1};
let mut hasher = Md2::new();
let mut hasher = Md4::new();
let mut hasher = Md5::new();
let mut hasher = Sha1::new();
```
**Correct (Rust - SHA256 hashing):**
```rust
use sha2::{Sha256};
let mut hasher = Sha256::new();
```
---
### Remediation Summary
| Language | Weak Algorithm | Secure Alternative |
|------------|----------------|-------------------|
| Python | `hashlib.md5`, `hashlib.sha1` | `hashlib.sha256`, `hashlib.sha512` |
| Python | `DES.new()` | `AES.new()` with EAX/GCM mode |
| JavaScript | `createHash("md5")` | `createHash("sha256")` |
| Java | `getInstance("MD5")`, `getInstance("SHA-1")` | `getInstance("SHA-512")` |
| Java | `getInstance("DES")` | `getInstance("AES/GCM/NoPadding")` |
| Go | `crypto/md5`, `crypto/sha1` | `crypto/sha256`, `crypto/sha512` |
| Go | `crypto/des` | `crypto/aes` |
### Best Practices
1. **Hashing:** Use SHA-256 or SHA-512 for general hashing. For passwords, use bcrypt, scrypt, or Argon2.
2. **Encryption:** Use AES with authenticated modes (GCM, EAX). Avoid ECB mode.
3. **Key sizes:** RSA keys should be at least 2048 bits. AES keys should be 256 bits.
4. **Random numbers:** Use cryptographically secure random number generators for security-sensitive operations.
skills/code-security/rules/insecure-deserialization.md
+110 -1592
View File
File diff suppressed because it is too large Load Diff
skills/code-security/rules/insecure-transport.md
+58 -830
View File
File diff suppressed because it is too large Load Diff
skills/code-security/rules/kubernetes.md
+58 -578
View File
@@ -10,372 +10,103 @@ This guide provides security best practices for Kubernetes YAML configurations.
Key Security Principles:
1. Least Privilege: Containers should run with minimal permissions and as non-root users
2. Isolation: Limit host namespace sharing (PID, network, IPC) to prevent container escapes
3. Immutability: Use read-only filesystems to prevent runtime modifications
4. Secure Communications: Always verify TLS certificates for encrypted connections
5. Secrets Management: Never store secrets directly in configuration files
6. RBAC: Apply principle of least privilege to cluster roles and permissions
3. Secrets Management: Never store secrets directly in configuration files
**Incorrect (Pod - security context missing allowPrivilegeEscalation):**
### Privileged Containers
Running containers in privileged mode grants full access to the host, bypassing security boundaries.
**Incorrect:**
```yaml
apiVersion: v1
kind: Pod
spec:
containers:
# ruleid: allow-privilege-escalation-no-securitycontext
- name: nginx
image: nginx
```
**Incorrect (Pod - privilege escalation explicitly enabled):**
```yaml
apiVersion: v1
kind: Pod
spec:
containers:
- name: redis
image: redis
securityContext:
# ruleid: allow-privilege-escalation-true
allowPrivilegeEscalation: true
```
**Incorrect (Pod - security context exists but missing allowPrivilegeEscalation):**
```yaml
apiVersion: v1
kind: Pod
spec:
containers:
- name: postgres
image: postgres
# ruleid: allow-privilege-escalation
securityContext:
runAsUser: 1000
runAsGroup: 3000
fsGroup: 2000
```
**Correct (Pod - privilege escalation explicitly disabled):**
```yaml
apiVersion: v1
kind: Pod
spec:
containers:
# ok: allow-privilege-escalation
- name: haproxy
image: haproxy
securityContext:
allowPrivilegeEscalation: false
```
**Incorrect (Pod - no security context at pod level and no runAsNonRoot at container level):**
```yaml
apiVersion: v1
kind: Pod
# ruleid: run-as-non-root
spec:
containers:
- name: nginx
image: nginx
- name: postgres
image: postgres
securityContext:
runAsUser: 1000
runAsGroup: 3000
fsGroup: 2000
- name: haproxy
image: haproxy
```
**Incorrect (Pod - runAsNonRoot explicitly set to false at pod level):**
```yaml
apiVersion: v1
kind: Pod
spec:
securityContext:
# ruleid: run-as-non-root-unsafe-value
runAsNonRoot: false
containers:
- name: redis
image: redis
- name: haproxy
image: haproxy
```
**Incorrect (Pod - runAsNonRoot explicitly set to false at container level):**
```yaml
apiVersion: v1
kind: Pod
spec:
containers:
- name: redis
image: redis
securityContext:
# ruleid: run-as-non-root-unsafe-value
runAsNonRoot: false
```
**Incorrect (Pod - security context at pod level missing runAsNonRoot):**
```yaml
apiVersion: v1
kind: Pod
spec:
# ruleid: run-as-non-root-security-context-pod-level
securityContext:
runAsGroup: 3000
containers:
- name: nginx
image: nginx
- name: postgres
image: postgres
securityContext:
runAsUser: 1000
runAsGroup: 3000
fsGroup: 2000
- name: haproxy
image: haproxy
```
**Incorrect (Pod - container security context missing runAsNonRoot when other containers have it):**
```yaml
apiVersion: v1
kind: Pod
spec:
containers:
# this is ok because there is no security context, requires different fix, so different rule
# ok: run-as-non-root-container-level
- name: nginx
image: nginx
- name: postgres
image: postgres
# ruleid: run-as-non-root-container-level
securityContext:
runAsUser: 1000
runAsGroup: 3000
fsGroup: 2000
- name: haproxy
image: haproxy
# ok: run-as-non-root-container-level
securityContext:
runAsNonRoot: true
```
**Incorrect (Pod - container missing security context when other containers have runAsNonRoot):**
```yaml
apiVersion: v1
kind: Pod
spec:
containers:
- name: nginx
# ruleid: run-as-non-root-container-level-missing-security-context
image: nginx
- name: postgres
image: postgres
# ok: run-as-non-root-container-level-missing-security-context
securityContext:
runAsUser: 1000
runAsGroup: 3000
fsGroup: 2000
- name: haproxy
image: haproxy
# ok: run-as-non-root-container-level-missing-security-context
securityContext:
runAsNonRoot: true
```
**Correct (Pod - runAsNonRoot set at pod level):**
```yaml
apiVersion: v1
kind: Pod
spec:
# ok: run-as-non-root
securityContext:
runAsNonRoot: true
containers:
- name: nginx
image: nginx
- name: postgres
image: postgres
securityContext:
runAsUser: 1000
runAsGroup: 3000
fsGroup: 2000
- name: haproxy
image: haproxy
```
**Correct (Pod - runAsNonRoot set at container level):**
```yaml
apiVersion: v1
kind: Pod
spec:
containers:
- name: haproxy
image: haproxy
securityContext:
# ok: run-as-non-root-unsafe-value
runAsNonRoot: true
```
**Incorrect (Pod - privileged mode at pod spec level):**
```yaml
apiVersion: v1
kind: Pod
spec:
# ruleid: privileged-container
privileged: true
containers:
- name: nginx
image: nginx
```
**Incorrect (Pod - privileged mode at container level):**
```yaml
apiVersion: v1
kind: Pod
spec:
containers:
# ruleid: privileged-container
- name: nginx
image: nginx
securityContext:
privileged: true
```
**Correct (Pod - privileged mode disabled):**
**Correct:**
```yaml
apiVersion: v1
kind: Pod
spec:
containers:
# ok: privileged-container
- name: redis
image: redis
securityContext:
privileged: false
```
**Correct (Pod - no privileged setting defaults to false):**
### Run as Non-Root
Containers should never run as root to limit the impact of container escapes.
**Incorrect:**
```yaml
apiVersion: v1
kind: Pod
spec:
securityContext:
runAsNonRoot: false
containers:
# ok: privileged-container
- name: postgres
image: postgres
- name: redis
image: redis
```
**Incorrect (Pod - no readOnlyRootFilesystem setting):**
**Correct:**
```yaml
apiVersion: v1
kind: Pod
spec:
securityContext:
runAsNonRoot: true
containers:
# ruleid: writable-filesystem-container
- name: nginx
image: nginx
```
**Incorrect (Pod - security context without readOnlyRootFilesystem):**
### Privilege Escalation
Prevent processes from gaining more privileges than their parent process.
**Incorrect:**
```yaml
apiVersion: v1
kind: Pod
spec:
containers:
# ruleid: writable-filesystem-container
- name: postgres
image: postgres
securityContext:
runAsUser: 1000
runAsGroup: 3000
fsGroup: 2000
```
**Incorrect (Pod - readOnlyRootFilesystem explicitly set to false):**
```yaml
apiVersion: v1
kind: Pod
spec:
containers:
# ruleid: writable-filesystem-container
- name: redis
image: redis
securityContext:
readOnlyRootFilesystem: false
allowPrivilegeEscalation: true
```
**Correct (Pod - read-only root filesystem enabled):**
**Correct:**
```yaml
apiVersion: v1
kind: Pod
spec:
containers:
# ok: writable-filesystem-container
- name: haproxy
image: haproxy
securityContext:
readOnlyRootFilesystem: true
allowPrivilegeEscalation: false
```
**Incorrect (Pod - seccomp profile set to unconfined):**
### Host PID Namespace
```yaml
apiVersion: v1
kind: Pod
spec:
containers:
# ok: seccomp-confinement-disabled
- name: nginx
image: nginx
# ok: seccomp-confinement-disabled
- name: postgres
image: postgres
securityContext:
runAsUser: 1000
runAsGroup: 3000
fsGroup: 2000
# ruleid: seccomp-confinement-disabled
- name: redis
image: redis
securityContext:
seccompProfile: unconfined
```
Sharing the host PID namespace allows containers to see and interact with all processes on the host.
**Correct (Pod - no explicit seccomp disable uses default):**
```yaml
apiVersion: v1
kind: Pod
spec:
containers:
# ok: seccomp-confinement-disabled
- name: nginx
image: nginx
securityContext:
runAsNonRoot: true
```
**Incorrect (Pod - host PID namespace enabled):**
**Incorrect:**
```yaml
apiVersion: v1
@@ -383,14 +114,13 @@ kind: Pod
metadata:
name: view-pid
spec:
# ruleid: hostpid-pod
hostPID: true
containers:
- name: nginx
image: nginx
```
**Correct (Pod - no hostPID setting defaults to false):**
**Correct:**
```yaml
apiVersion: v1
@@ -403,22 +133,25 @@ spec:
image: nginx
```
**Incorrect (Pod - host network namespace enabled):**
### Host Network Namespace
Sharing the host network namespace exposes the host network stack to the container.
**Incorrect:**
```yaml
apiVersion: v1
kind: Pod
metadata:
name: view-pid
name: view-network
spec:
# ruleid: hostnetwork-pod
hostNetwork: true
containers:
- name: nginx
image: nginx
```
**Correct (Pod - no hostNetwork setting defaults to false):**
**Correct:**
```yaml
apiVersion: v1
@@ -431,22 +164,25 @@ spec:
image: nginx
```
**Incorrect (Pod - host IPC namespace enabled):**
### Host IPC Namespace
Sharing the host IPC namespace allows containers to access shared memory on the host.
**Incorrect:**
```yaml
apiVersion: v1
kind: Pod
metadata:
name: view-pid
name: view-ipc
spec:
# ruleid: hostipc-pod
hostIPC: true
containers:
- name: nginx
image: nginx
```
**Correct (Pod - no hostIPC setting defaults to false):**
**Correct:**
```yaml
apiVersion: v1
@@ -459,13 +195,15 @@ spec:
image: nginx
```
**Incorrect (Pod - Docker socket mounted as hostPath):**
### Docker Socket Exposure
Mounting the Docker socket gives containers full control over the Docker daemon.
**Incorrect:**
```yaml
apiVersion: v1
kind: Pod
metadata:
name: test-pd
spec:
containers:
- image: gcr.io/google_containers/test-webserver
@@ -475,19 +213,16 @@ spec:
name: docker-sock-volume
volumes:
- name: docker-sock-volume
# ruleid: exposing-docker-socket-hostpath
hostPath:
type: File
path: /var/run/docker.sock
```
**Correct (Pod - no Docker socket mounting):**
**Correct:**
```yaml
apiVersion: v1
kind: Pod
metadata:
name: test-pd
spec:
containers:
- image: gcr.io/google_containers/test-webserver
@@ -500,7 +235,11 @@ spec:
emptyDir: {}
```
**Incorrect (Secret - secrets stored in config file):**
### Secrets in Config Files
Never store secrets directly in configuration files. Use external secrets management.
**Incorrect:**
```yaml
apiVersion: v1
@@ -509,20 +248,13 @@ metadata:
name: mysecret
type: Opaque
data:
# ruleid: secrets-in-config-file
USER NAME: Y2FsZWJraW5uZXk=
# ok: secrets-in-config-file
UUID: {UUID}
# ruleid: secrets-in-config-file
USERNAME: Y2FsZWJraW5uZXk=
PASSWORD: UzNjcmV0UGEkJHcwcmQ=
# ok: secrets-in-config-file
SERVER: cHJvZA==
```
**Correct (Secret - use Sealed Secrets or external secrets management):**
**Correct (use Sealed Secrets or external secrets management):**
```yaml
# Using Bitnami Sealed Secrets
apiVersion: bitnami.com/v1alpha1
kind: SealedSecret
metadata:
@@ -531,255 +263,3 @@ spec:
encryptedData:
password: AgBy8hCi8...encrypted...
```
**Incorrect (Config - TLS verification disabled for cluster):**
```yaml
apiVersion: v1
clusters:
# ruleid: skip-tls-verify-cluster
- cluster:
server: https://192.168.0.100:8443
insecure-skip-tls-verify: true
name: minikube1
contexts:
- context:
cluster: minikube
user: minikube
name: minikube
current-context: minikube
kind: Config
```
**Correct (Config - TLS verification enabled):**
```yaml
apiVersion: v1
clusters:
# ok: skip-tls-verify-cluster
- cluster:
server: https://192.168.0.101:8443
name: minikube2
contexts:
- context:
cluster: minikube
user: minikube
name: minikube
current-context: minikube
kind: Config
users:
- name: minikube
user:
client-certificate: client.crt
client-key: client.key
```
**Incorrect (APIService - TLS verification disabled):**
```yaml
apiVersion: apiregistration.k8s.io/v1beta1
kind: APIService
metadata:
name: v1beta1.metrics.k8s.io
# ruleid: skip-tls-verify-service
spec:
service:
name: metrics-server
namespace: kube-system
group: metrics.k8s.io
version: v1beta1
insecureSkipTLSVerify: true
groupPriorityMinimum: 100
versionPriority: 100
```
**Correct (APIService - TLS verification enabled):**
```yaml
apiVersion: apiregistration.k8s.io/v1beta1
kind: APIService
metadata:
name: v1beta1.metrics.k8s.io
spec:
service:
name: metrics-server
namespace: kube-system
group: metrics.k8s.io
version: v1beta1
caBundle: <base64-encoded-ca-cert>
groupPriorityMinimum: 100
versionPriority: 100
```
**Incorrect (ClusterRole - wildcard permissions on core API):**
```yaml
apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRole
metadata:
name: bad-role
rules:
# ok: legacy-api-clusterrole-excessive-permissions
- apiGroups:
- apps
resources:
- "*"
verbs:
- "*"
- apiGroups:
- ""
resources:
# ruleid: legacy-api-clusterrole-excessive-permissions
- "*"
verbs:
# ruleid: legacy-api-clusterrole-excessive-permissions
- "*"
```
**Incorrect (ClusterRole - inline wildcard permissions):**
```yaml
apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRole
metadata:
name: bad-role-inline
rules:
- apiGroups: [""]
# ruleid: legacy-api-clusterrole-excessive-permissions
resources: ["*"]
# ruleid: legacy-api-clusterrole-excessive-permissions
verbs: ["*"]
```
**Correct (ClusterRole - explicit permissions):**
```yaml
apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRole
metadata:
name: good-role
rules:
# ok: legacy-api-clusterrole-excessive-permissions
- apiGroups:
- ""
resources:
- configmaps
verbs:
- get
- list
- watch
- create
- update
- patch
- delete
```
**Correct (ClusterRole - wildcard resources but limited verbs):**
```yaml
apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRole
metadata:
name: read-only-role
rules:
# ok: legacy-api-clusterrole-excessive-permissions
- apiGroups:
- ""
resources: ["*"]
verbs:
- list
```
**Incorrect (Deployment - FLASK_ENV set to development):**
```yaml
apiVersion: apps/v1
kind: Deployment
metadata:
name: backend
labels:
tags.datadoghq.com/env: dev
spec:
template:
metadata:
labels:
tags.datadoghq.com/env: dev
spec:
initContainers:
- name: migrate-db
env:
- name: SQLALCHEMY_DATABASE_URI
valueFrom:
secretKeyRef:
name: backend-secrets
key: SQLALCHEMY_DATABASE_URI
# ruleid: flask-debugging-enabled
- name: FLASK_ENV
value: development
```
**Correct (Deployment - FLASK_ENV set to non-development value):**
```yaml
apiVersion: apps/v1
kind: Deployment
metadata:
name: backend
spec:
template:
spec:
containers:
- name: backend
env:
# ok: flask-debugging-enabled
- name: FLASK_ENV
value: dev
```
**Incorrect (Deployment - fractional CPU limit causing throttling):**
```yaml
kind: Deployment
apiVersion: apps/v1
metadata:
name: mumbledj
namespace: mumble
spec:
template:
spec:
containers:
- name: app
image: underyx/mumbledj
resources:
limits:
# ruleid: no-fractional-cpu-limits
cpu: 100m
memory: 64Mi
requests:
# ok: no-fractional-cpu-limits
cpu: 20m
memory: 32Mi
```
**Correct (Deployment - full CPU unit limits):**
```yaml
kind: Deployment
apiVersion: apps/v1
metadata:
name: app
spec:
template:
spec:
containers:
- name: app
image: panubo/sshd:1.1.0
resources:
limits:
# ok: no-fractional-cpu-limits
cpu: 1000m
memory: 512Mi
requests:
cpu: 10m
memory: 8Mi
```
skills/code-security/rules/maintainability.md
+37 -1148
View File
File diff suppressed because it is too large Load Diff
skills/code-security/rules/memory-safety.md
+54 -448
View File
@@ -5,43 +5,40 @@ impact: CRITICAL
## Ensure Memory Safety
Memory safety vulnerabilities are among the most critical security issues in software development. They can lead to arbitrary code execution, data corruption, denial of service, and information disclosure. This guide covers common memory safety issues including buffer overflows, use-after-free, double-free, format string vulnerabilities, and out-of-bounds memory access.
Memory safety vulnerabilities are among the most critical security issues in software development. They can lead to arbitrary code execution, data corruption, denial of service, and information disclosure. This guide covers common memory safety issues in C/C++ including double-free, use-after-free, and buffer overflow vulnerabilities.
**Incorrect (C - double free vulnerability, CWE-415):**
### Double Free (CWE-415)
Freeing memory twice can cause memory corruption, crashes, or allow attackers to execute arbitrary code.
**Incorrect:**
```c
int bad_code1() {
int bad_code() {
char *var = malloc(sizeof(char) * 10);
free(var);
// ruleid: double-free
free(var);
free(var); // Double free vulnerability
return 0;
}
```
**Correct (C - set pointer to NULL after free):**
**Correct:**
```c
int okay_code1() {
int safe_code() {
char *var = malloc(sizeof(char) * 10);
free(var);
var = NULL;
// ok: double-free
free(var);
return 0;
}
int okay_code2() {
char *var = malloc(sizeof(char) * 10);
free(var);
var = malloc(sizeof(char) * 10);
// ok: double-free
free(var);
var = NULL; // Set to NULL after free
free(var); // Safe: freeing NULL is a no-op
return 0;
}
```
**Incorrect (C - use after free vulnerability, CWE-416):**
### Use After Free (CWE-416)
Accessing memory after it has been freed can lead to crashes, data corruption, or code execution.
**Incorrect:**
```c
typedef struct name {
@@ -49,118 +46,16 @@ typedef struct name {
void (*func)(char *str);
} NAME;
int bad_code1() {
int bad_code() {
NAME *var;
var = (NAME *)malloc(sizeof(struct name));
free(var);
// ruleid: use-after-free
var->func("use after free");
return 0;
}
int bad_code2() {
NAME *var;
var = (NAME *)malloc(sizeof(struct name));
free(var);
// ruleid: use-after-free
other_func(var->myname);
return 0;
}
int bad_code3(){
struct NAME *var;
var = malloc(sizeof(s_auth));
free(var);
// ruleid: use-after-free
if(var->auth){
printf("you have logged in already");
}
else{
printf("you do not have the permision to log in.");
}
return 0;
}
int bad_code4(){
int initial = 1000;
struct lv *lv = malloc(sizeof(*lv));
lv->length = initial;
lv->value = malloc(initial);
free(lv);
// ruleid: use-after-free
free(lv->value);
return 0;
}
int bad_code6() {
NAME *var;
var = (NAME *)malloc(sizeof(struct name));
free(var);
// ruleid: use-after-free
(*var).func("use after free");
return 0;
}
int bad_code7() {
char *var;
char buf[10];
var = (char *)malloc(100);
free(var);
// ruleid: use-after-free
char buf[0] = var[0];
var->func("use after free"); // Accessing freed memory
return 0;
}
```
**Correct (C - safe use after free patterns):**
```c
int okay_code1() {
NAME *var;
var = (NAME *)malloc(sizeof(struct name));
free(var);
var = NULL;
// This will segmentation fault
// ok: use-after-free
var->func("use after free");
return 0;
}
int okay_code2() {
NAME *var;
var = (NAME *)malloc(sizeof(struct name));
free(var);
var = NULL;
// This will segmentation fault
// ok: use-after-free
other_func(var->myname);
return 0;
}
int ok_code4(){
int initial = 1000;
struct lv *lv = malloc(sizeof(*lv));
lv->length = initial;
lv->value = malloc(initial);
// ok: use-after-free
free(lv->value);
// ok: use-after-free
free(lv);
return 0;
}
int ok_code6() {
NAME *var;
var = (NAME *)malloc(sizeof(struct name));
free(var);
var = (NAME *)malloc(sizeof(struct name));
// ok: use-after-free
(*var).func("use after free");
return 0;
}
```
**Incorrect (C - function use after free, CWE-416):**
**Correct:**
```c
typedef struct name {
@@ -168,353 +63,64 @@ typedef struct name {
void (*func)(char *str);
} NAME;
int bad_code1() {
int safe_code() {
NAME *var;
char buf[10];
var = (NAME *)malloc(sizeof(struct name));
free(var);
// ruleid: function-use-after-free
strcpy(buf, (char*)var);
// ruleid: function-use-after-free
other_func((char*)(*var));
// ruleid: function-use-after-free
other_func((char*)var[0]);
// ruleid: function-use-after-free
var->func(var->myname);
return 0;
}
int bad_code2() {
NAME *var;
char buf[10];
var = (NAME *)malloc(sizeof(struct name));
free(var);
// ruleid: function-use-after-free
strcpy(buf, (char*)*var);
// ruleid: function-use-after-free
other_func((char*)var);
// ruleid: function-use-after-free
other_func((char*)var->myname);
var = NULL; // Prevents accidental reuse
// Any access to var now causes immediate crash (easier to debug)
return 0;
}
```
**Correct (C - safe function use after free patterns):**
### Buffer Overflow (CWE-119, CWE-120)
Writing beyond buffer boundaries can overwrite adjacent memory, leading to crashes or code execution.
**Incorrect:**
```c
int okay_code1() {
NAME *var;
var = (NAME *)malloc(sizeof(struct name));
free(var);
var = NULL;
// This will segmentation fault
// ok: function-use-after-free
other_func((char*)var);
other_func((char*)var->myname);
other_func((char*)*var);
return 0;
}
int okay_code2() {
NAME *var;
var = (NAME *)malloc(sizeof(struct name));
free(var);
var = NULL;
var = (NAME *)malloc(sizeof(struct name));
// This will segmentation fault
// ok: function-use-after-free
other_func((char*)var);
other_func((char*)var->myname);
other_func((char*)*var);
return 0;
void bad_code(char *user_input) {
char buffer[64];
strcpy(buffer, user_input); // No bounds checking
}
```
**Incorrect (C - insecure format string functions, CWE-134):**
**Correct:**
```c
void bad_vsprintf(int argc, char **argv) {
char format[256];
//ruleid: insecure-use-printf-fn
strncpy(format, argv[1], 255);
char buffer[100];
vsprintf (buffer,format, args);
//ruleid: insecure-use-printf-fn
vsprintf(buffer, argv[1], args);
}
void bad_sprintf(int argc, char **argv) {
char format[256];
int a = 10, b = 20, c=30;
//ruleid: insecure-use-printf-fn
strcpy(format, argv[1]);
char buffer[200];
sprintf(buffer, format, a, b, c);
char buffer[256];
int i = 3;
//ruleid: insecure-use-printf-fn
sprintf(buffer, argv[2], a, b, c);
}
void bad_printf() {
//ruleid: insecure-use-printf-fn
printf(argv[2], 1234);
char format[300];
//ruleid: insecure-use-printf-fn
strcpy(format, argv[1]);
printf(format, 1234);
void safe_code(char *user_input) {
char buffer[64];
strncpy(buffer, user_input, sizeof(buffer) - 1);
buffer[sizeof(buffer) - 1] = '\0'; // Ensure null termination
}
```
**Correct (C - safe format string usage):**
### Format String Vulnerabilities (CWE-134)
Using user-controlled format strings can allow attackers to read or write arbitrary memory.
**Incorrect:**
```c
void safe_vsprintf(int argc, char **argv) {
//ok: insecure-use-printf-fn
vsprintf("%s\n",argv[0]);
//ok: insecure-use-printf-fn
vsnprintf(buffer, format, args);
}
void safe_sprintf(int argc, char **argv) {
//ok: insecure-use-printf-fn
sprintf("%s\n",argv[0]);
//ok: insecure-use-printf-fn
snprintf(buffer, format, a,b,c);
}
void safe_printf() {
//ok: insecure-use-printf-fn
printf("hello");
//ok: insecure-use-printf-fn
printf("%s\n",argv[0]);
void bad_printf(char *user_input) {
printf(user_input); // User controls format string
}
```
**Incorrect (JavaScript - Buffer noassert out-of-bounds, CWE-119):**
**Correct:**
```javascript
// ruleid:detect-buffer-noassert
a.readUInt8(0, true)
// ruleid:detect-buffer-noassert
a.writeFloatLE(0, true)
```
**Correct (JavaScript - Buffer with bounds checking):**
```javascript
// ok:detect-buffer-noassert
a.readUInt8(0)
// ok:detect-buffer-noassert
a.readUInt8(0, false)
```
**Incorrect (JavaScript - unsafe format string, CWE-134):**
```javascript
const util = require('util')
function test1(data) {
const {user, ip} = data
foobar(user)
// ruleid: unsafe-formatstring
console.log("Unauthorized access attempt by " + user, ip);
}
function test2(data) {
const {user, ip} = data
foobar(user)
const logs = `Unauthorized access attempt by ${user}`
// ruleid: unsafe-formatstring
console.log(logs, ip);
}
function test3(data) {
const {user, ip} = data
foobar(user)
const logs = `Unauthorized access attempt by ${user} %d`
// ruleid: unsafe-formatstring
return util.format(logs, ip);
```c
void safe_printf(char *user_input) {
printf("%s", user_input); // Format string is fixed
}
```
**Correct (JavaScript - safe format string usage):**
### Prevention Best Practices
```javascript
const util = require('util')
function okTest1(data) {
const {user, ip} = data
foobar(user)
const logs = `Unauthorized access attempt by user`
// ok: unsafe-formatstring
console.log(logs, ip);
}
function okTest2(data) {
const {user, ip} = data
foobar(user)
// ok: unsafe-formatstring
console.log("Unauthorized access attempt by " + user);
}
function okTest3(data) {
const {user, ip} = data
foobar(user)
// ok: unsafe-formatstring
return util.format("Unauthorized access attempt by %d", ip);
}
```
**Incorrect (C# - MemoryMarshal CreateSpan out-of-bounds read, CWE-125):**
```csharp
namespace MemMarshalCreateSpan {
public class MemMarshalCreateSpan {
public void MarshalTest() {
// ruleid: memory-marshal-create-span
Span<T> ToSpan() => MemoryMarshal.CreateSpan(ref _e0, 1);
// ruleid: memory-marshal-create-span
Span<T> ToSpan() => MemoryMarshal.CreateReadOnlySpan(ref _e0, 2);
// ruleid: memory-marshal-create-span
Span<byte> span = MemoryMarshal.CreateSpan(ref Unsafe.AsRef(writer.Span.GetPinnableReference()), 4);
// ruleid: memory-marshal-create-span
Span<byte> span = MemoryMarshal.CreateReadOnlySpan(ref Unsafe.AsRef(writer.Span.GetPinnableReference()), 8);
}
}
}
```
**Correct (C# - safe Span creation with bounds checking):**
Use standard Span creation methods with bounds checking, or validate the length parameter before calling MemoryMarshal methods.
**Incorrect (PHP - base_convert loses precision, CWE-190):**
```php
<?php
// ruleid: base-convert-loses-precision
$token = base_convert(bin2hex(hash('sha256', uniqid(mt_rand(), true), true)), 16, 36);
// ruleid: base-convert-loses-precision
base_convert(hash_hmac('sha256', $command . ':' . $token, $secret), 16, 36);
// ruleid: base-convert-loses-precision
$randString = base_convert(sha1(uniqid(mt_rand(), true)), 16, 36);
// ruleid: base-convert-loses-precision
$uniqueId = substr(base_convert(md5(uniqid(rand(), true)), 16, 36), 1, 20);
// ruleid: base-convert-loses-precision
$token = base_convert(sha1($i),7, 36);
// ruleid: base-convert-loses-precision
$salt = base_convert(bin2hex(random_bytes(20)), 16, 36);
$stringHash = substr(md5($string), 0, 8);
// ruleid: base-convert-loses-precision
base_convert($stringHash, 16, 10);
// ruleid: base-convert-loses-precision
$seed = base_convert(md5(microtime().$_SERVER['DOCUMENT_ROOT']), 16, $numeric ? 10 : 35);
$bytes = random_bytes(32);
// ruleid: base-convert-loses-precision
base_convert(bin2hex($bytes), 16, 36);
// ruleid: base-convert-loses-precision
base_convert(bin2hex(openssl_random_pseudo_bytes(8)), 16, 36);
// ruleid: base-convert-loses-precision
$salt = base_convert(bin2hex($this->security->get_random_bytes(20)), 16,36);
```
**Correct (PHP - safe base_convert usage with small numbers):**
```php
<?php
// ok: base-convert-loses-precision
var_dump(base_convert("0775", 8, 10));
// ok: base-convert-loses-precision
$token = 'gleez_profiler/'.base_convert($counter++, 10, 32);
// ok: base-convert-loses-precision
$color1Index = base_convert(substr($uid, 0, 2), 16, 10) % $totalColors;
// ok: base-convert-loses-precision
$id_converted = base_convert($row, 10, 36);
// ok: base-convert-loses-precision
$value = base_convert(substr($value, 2), 16, 10);
// ok: base-convert-loses-precision
base_convert(rand(1, 1000000000), 10, 36);
// taking only 7 hex chars makes it fit into a 64-bit integer
$stringHash = substr(md5($string), 0, 7);
// ok: base-convert-loses-precision
base_convert($stringHash, 16, 10);
// ok: base-convert-loses-precision
base_convert(bin2hex(iconv('UTF-8', 'UCS-4', $m)), 16, 10);
// ok: base-convert-loses-precision
$currentByteBits = str_pad(base_convert(bin2hex(fread($fp,1)), 16, 2),8,'0',STR_PAD_LEFT);
// ok: base-convert-loses-precision
base_convert(bin2hex(random_bytes(7)), 16, 36);
```
**Incorrect (Python/Flask - API method string format injection, CWE-134):**
```python
import requests
class FOO(resource):
method_decorators = decorator()
# ruleid:flask-api-method-string-format
def get(self, arg1):
print("foo")
string = "foo".format(arg1)
foo = requests.get(string)
# ruleid:flask-api-method-string-format
def get2(self,arg2):
someFn()
bar = requests.get("foo".format(arg2))
```
**Correct (Python/Flask - safe API method patterns):**
```python
import requests
class FOO(resource):
method_decorators = decorator()
# ok:flask-api-method-string-format
def get(self, somearg):
createRecord(somearg)
# ok:flask-api-method-string-format
def get(self, somearg):
otherFunc("hello world")
```
1. **Set pointers to NULL after freeing** - Prevents use-after-free and double-free
2. **Use bounded string functions** - `strncpy`, `snprintf` instead of `strcpy`, `sprintf`
3. **Never use user input as format strings** - Always use fixed format strings
4. **Validate array indices** - Check bounds before accessing arrays
5. **Use static analysis tools** - Semgrep, Coverity, or similar to detect issues
6. **Consider memory-safe languages** - Rust, Go, or managed languages where appropriate
skills/code-security/rules/path-traversal.md
+90 -869
View File
File diff suppressed because it is too large Load Diff
skills/code-security/rules/performance.md
+44 -343
View File
@@ -5,362 +5,78 @@ impact: LOW
# Performance Best Practices
This document covers performance optimizations and best practices to write efficient code. These rules identify patterns that cause unnecessary computational overhead, extra database queries, memory inefficiency, or render bottlenecks.
## Table of Contents
- [Python](#python)
- [Django](#django)
- [SQLAlchemy](#sqlalchemy)
- [Ruby](#ruby)
- [Rails](#rails)
- [C](#c)
- [C#](#c-1)
- [TypeScript/JavaScript](#typescriptjavascript)
- [React](#react)
- [OCaml](#ocaml)
This document covers performance optimizations to write efficient code. These rules identify patterns that cause unnecessary computational overhead, extra database queries, or memory inefficiency.
---
## Python
### Django
### Django - Access Foreign Keys Directly
#### Access Foreign Keys Directly
You should use `ITEM.user_id` rather than `ITEM.user.id` to prevent running an extra query. Accessing `.user.id` causes Django to fetch the entire related User object just to get the ID, when the foreign key ID is already available on the model.
Reference: [Django Documentation - Use foreign key values directly](https://docs.djangoproject.com/en/5.0/topics/db/optimization/#use-foreign-key-values-directly)
Use `ITEM.user_id` rather than `ITEM.user.id` to prevent running an extra query. Accessing `.user.id` causes Django to fetch the entire related User object just to get the ID, when the foreign key ID is already available on the model.
**INCORRECT** - Extra query to fetch related object:
```python
from django.http import HttpResponse
from models import User
def other():
# ruleid: access-foreign-keys
print(User.user.id)
def get_user_id(item):
return item.user.id
```
**CORRECT** - Use request.user.id which is already loaded:
**CORRECT** - Use the foreign key directly:
```python
from django.http import HttpResponse
from models import User
def cool_view(request):
# ok: access-foreign-keys
return HttpResponse({"user_id": request.user.id})
class View(APIView):
def get_queryset(self):
# ok: access-foreign-keys
print(self.request.user.id)
return super().get_queryset()
def get_user_id(item):
return item.user_id
```
---
### SQLAlchemy
### SQLAlchemy - Use count() Instead of len(all())
#### Use count() Instead of len(all())
Using `QUERY.count()` instead of `len(QUERY.all())` sends less data to the client since the SQLAlchemy method is performed server-side. The `len(all())` approach fetches all records into memory just to count them.
Using `QUERY.count()` instead of `len(QUERY.all())` sends less data to the client since the count is performed server-side. The `len(all())` approach fetches all records into memory just to count them.
**INCORRECT** - Fetches all records into memory:
```python
# ruleid:len-all-count
len(persons.all())
total = len(persons.all())
```
**CORRECT** - Count performed server-side:
```python
# ok:len-all-count
persons.count()
total = persons.count()
```
#### Batch Database Operations
---
Rather than adding one element at a time, consider batch loading to improve performance. Each individual `db.session.add()` in a loop can trigger separate database operations.
### SQLAlchemy - Batch Database Operations
Rather than adding one element at a time, use batch loading to improve performance. Each individual `db.session.add()` in a loop can trigger separate database operations.
**INCORRECT** - Adding one at a time in a loop:
```python
# ruleid:batch-import
for song in songs:
db.session.add(song)
```
**CORRECT** - Batch add all at once:
```python
# ok:batch-import
db.session.add_all(songs)
```
---
## Ruby
## JavaScript/TypeScript
### Rails
### React - Define Styled Components at Module Level
#### Add Indexes for Foreign Keys
By declaring a styled component inside the render method, you dynamically create a new component on every render. This forces React to discard and re-calculate that part of the DOM subtree on each render, leading to performance bottlenecks.
Foreign key columns (columns ending in `_id`) should have database indexes to improve query performance. Without an index, queries filtering or joining on foreign keys require full table scans.
Reference: [Why Your Database Needs Indexes](https://archive.is/i7SLO)
**INCORRECT** - Foreign key column without index:
```ruby
class CreateProducts < ActiveRecord::Migration[7.0]
def change
# ruleid: ruby-rails-performance-indexes-are-beneficial
add_column :users3, :email3_id, :integer, foo: bar
add_index :users3, [:email2_id, :other_id], name: "asdf"
# ruleid: ruby-rails-performance-indexes-are-beneficial
add_column :users4, :email4_id, :integer, { other_stuff: :asdf }
# ruleid: ruby-rails-performance-indexes-are-beneficial
add_column :users4, :email4_id, :bigint, { other_stuff: :asdf }
end
end
```
**CORRECT** - Add index immediately after adding foreign key column:
```ruby
class CreateProducts < ActiveRecord::Migration[7.0]
def change
# ok: ruby-rails-performance-indexes-are-beneficial
add_column :users, :email_id, :integer
add_index :users, :email_id
# ok: ruby-rails-performance-indexes-are-beneficial
add_column :users2, :email2_id, :integer, foo: :bar
add_index :users2, :email2_id, name: "asdf"
end
end
```
---
## C
#### Use strcmp for String Comparison
Using `==` on `char*` performs pointer comparison, not string content comparison. Use `strcmp` instead to compare the actual string values.
**INCORRECT** - Pointer comparison instead of string comparison:
```c
#include <stddef.h>
#include <string.h>
int main()
{
char *s = "Hello";
// ruleid:c-string-equality
if (s == "World") {
return -1;
}
return 0;
}
```
**CORRECT** - Use strcmp for string content comparison:
```c
#include <stddef.h>
#include <string.h>
int main()
{
char *s = "Hello";
// ok:c-string-equality
if (strcmp(s, "World") == 0) {
return 1;
}
// ok:c-string-equality
if (!strcmp(s, "World")) {
return 1;
}
// ok:c-string-equality
if (s == 0) {
return 1;
}
// ok:c-string-equality
if (NULL == s) {
return 1;
}
return 0;
}
```
---
## C#
#### Use Structured Logging
String interpolation in log messages obscures the distinction between variables and the log message. Use structured logging instead, where the variables are passed as additional arguments and the interpolation is performed by the logging library. This reduces the possibility of log injection and makes it easier to search through logs.
CWE: CWE-117: Improper Output Neutralization for Logs
References:
- [NLog - How to use structured logging](https://github.com/NLog/NLog/wiki/How-to-use-structured-logging)
- [Benefits of Structured Logging vs Basic Logging](https://softwareengineering.stackexchange.com/questions/312197/benefits-of-structured-logging-vs-basic-logging)
**INCORRECT** - String interpolation in log messages:
```csharp
using Microsoft.Extensions.Logging;
using Serilog;
using NLog;
class Program
{
public static void SerilogSample()
{
using var serilog = new LoggerConfiguration().WriteTo.Console().CreateLogger();
var position = new { Latitude = 25, Longitude = 134 };
var elapsedMs = 34;
// ruleid: structured-logging
serilog.Information($"Processed {position} in {elapsedMs:000} ms.");
}
public static void MicrosoftSample()
{
var loggerFactory = LoggerFactory.Create(builder => {
builder.AddConsole();
}
);
var logger = loggerFactory.CreateLogger<Program>();
var position = new { Latitude = 25, Longitude = 134 };
var elapsedMs = 34;
// ruleid: structured-logging
logger.LogInformation($"Processed {position} in {elapsedMs:000} ms.");
}
public static void NLogSample()
{
var logger = NLog.LogManager.Setup().LoadConfiguration(builder => {
builder.ForLogger().WriteToConsole();
}).GetCurrentClassLogger();
var position = new { Latitude = 25, Longitude = 134 };
var elapsedMs = 34;
// ruleid: structured-logging
logger.Info($"Processed {position} in {elapsedMs:000} ms.");
// try with different name
var _LOG = logger;
// ruleid: structured-logging
_LOG.Info($"Processed {position} in {elapsedMs:000} ms.");
}
}
```
**CORRECT** - Pass variables as structured arguments:
```csharp
using Microsoft.Extensions.Logging;
using Serilog;
using NLog;
class Program
{
public static void SerilogSample()
{
using var serilog = new LoggerConfiguration().WriteTo.Console().CreateLogger();
var position = new { Latitude = 25, Longitude = 134 };
var elapsedMs = 34;
// ok: structured-logging
serilog.Information("Processed {@Position} in {Elapsed:000} ms.", position, elapsedMs);
}
public static void MicrosoftSample()
{
var loggerFactory = LoggerFactory.Create(builder => {
builder.AddConsole();
}
);
var logger = loggerFactory.CreateLogger<Program>();
var position = new { Latitude = 25, Longitude = 134 };
var elapsedMs = 34;
// ok: structured-logging
logger.LogInformation("Processed {@Position} in {Elapsed:000} ms.", position, elapsedMs);
}
public static void NLogSample()
{
var logger = NLog.LogManager.Setup().LoadConfiguration(builder => {
builder.ForLogger().WriteToConsole();
}).GetCurrentClassLogger();
var position = new { Latitude = 25, Longitude = 134 };
var elapsedMs = 34;
// ok: structured-logging
logger.Info("Processed {@Position} in {Elapsed:000} ms.", position, elapsedMs);
}
}
```
---
## TypeScript/JavaScript
### React
#### Define Styled Components at Module Level
By declaring a styled component inside the render method of a React component, you are dynamically creating a new component on every render. This means that React will have to discard and re-calculate that part of the DOM subtree on each subsequent render, instead of just calculating the difference of what changed between them. This leads to performance bottlenecks and unpredictable behavior.
Reference: [styled-components FAQ - Why should I avoid declaring styled-components in the render method](https://styled-components.com/docs/faqs#why-should-i-avoid-declaring-styled-components-in-the-render-method)
**INCORRECT** - Styled component declared inside function/class:
**INCORRECT** - Styled component declared inside function:
```tsx
import styled from "styled-components";
function FunctionalComponent() {
// ruleid: define-styled-components-on-module-level
const ArbitraryComponent3 = styled.div`
const StyledDiv = styled.div`
color: blue;
`
return <ArbitraryComponent3 />
}
function FunctionalComponent2() {
// ruleid: define-styled-components-on-module-level
const ArbitraryComponent3 = styled(FunctionalComponent)`
color: blue;
`
return <ArbitraryComponent3 />
}
class ClassComponent {
public render() {
// ruleid: define-styled-components-on-module-level
const ArbitraryComponent4 = styled.div`
color: blue;
`
return <ArbitraryComponent4 />
}
return <StyledDiv />
}
```
@@ -368,54 +84,39 @@ class ClassComponent {
```tsx
import styled from "styled-components";
// ok: define-styled-components-on-module-level
const ArbitraryComponent = styled.div`
color: blue;
`
// ok: define-styled-components-on-module-level
const ArbitraryComponent2 = styled(ArbitraryComponent)`
const StyledDiv = styled.div`
color: blue;
`
function FunctionalComponent() {
return <ArbitraryComponent />
return <StyledDiv />
}
```
---
## OCaml
### Avoid Unnecessary Operations in Loops
#### Use Empty List Check Instead of List.length
Check array length efficiently without traversing the entire collection.
Checking `List.length xs = 0` or `List.length xs > 0` is inefficient. `List.length` traverses the entire list to count elements. For checking if a list is empty or non-empty, compare directly against `[]`.
**INCORRECT** - Using List.length for empty check:
```ocaml
let test xs =
(* ruleid:ocamllint-length-list-zero *)
if List.length xs = 0
then 1
else 2
let test2 xs =
(* ruleid:ocamllint-length-more-than-zero *)
if List.length xs > 0
then 1
else 2
**INCORRECT** - Inefficient length check:
```javascript
if (items.length === 0) { /* empty */ }
```
**CORRECT** - Compare directly against empty list:
```ocaml
let test xs =
(* ok:ocamllint-length-list-zero *)
if xs = []
then 1
else 2
let test2 xs =
(* ok:ocamllint-length-more-than-zero *)
if xs <> []
then 1
else 2
**CORRECT** - Direct comparison when possible:
```javascript
if (!items.length) { /* empty */ }
```
For operations that require iterating, prefer built-in methods that short-circuit:
**INCORRECT** - Full iteration to find one item:
```javascript
const found = items.filter(x => x.id === targetId)[0];
```
**CORRECT** - Short-circuit on first match:
```javascript
const found = items.find(x => x.id === targetId);
```
skills/code-security/rules/prototype-pollution.md
+23 -457
View File
@@ -1,28 +1,18 @@
---
title: Prevent Prototype Pollution
impact: HIGH
impactDescription: Attackers can modify object prototypes to inject malicious properties, leading to privilege escalation, denial of service, or remote code execution
tags: security, prototype-pollution, mass-assignment, cwe-915
impactDescription: Attackers can modify object prototypes to inject malicious properties
tags: security, prototype-pollution, cwe-915
---
## Prevent Prototype Pollution
Prototype pollution is a vulnerability that occurs when an attacker can modify the prototype of a base object, such as `Object.prototype` in JavaScript. By adding or modifying attributes of an object prototype, it is possible to create attributes that exist on every object, or replace critical attributes with malicious ones (such as `hasOwnProperty`, `toString`, or `valueOf`).
Prototype pollution is a vulnerability that occurs when an attacker can modify the prototype of a base object, such as `Object.prototype` in JavaScript. This can create attributes that exist on every object or replace critical attributes with malicious ones.
This vulnerability class also includes mass assignment attacks in other languages, where attackers can set arbitrary attributes on models by manipulating request parameters.
**Mitigations:** Freeze prototypes with `Object.freeze(Object.prototype)`, use `Object.create(null)`, block `__proto__` and `constructor` keys, or use `Map` instead of objects.
**Possible mitigations:**
- Freeze the object prototype using `Object.freeze(Object.prototype)`
- Use objects without prototypes via `Object.create(null)`
- Block modifications to attributes that resolve to object prototype (`__proto__`, `constructor`)
- Use `Map` instead of plain objects for key-value storage
- In web frameworks, use strong parameter allowlisting to control which attributes can be set
**Incorrect (JavaScript - dynamic property assignment from user input):**
---
### Language: JavaScript / TypeScript
**Incorrect (vulnerable to prototype pollution via dynamic assignment):**
```javascript
app.get('/test/:id', (req, res) => {
let id = req.params.id;
@@ -30,500 +20,76 @@ app.get('/test/:id', (req, res) => {
if (!items) {
items = req.session.todos[id] = {};
}
// ruleid: prototype-pollution-assignment
items[req.query.name] = req.query.text;
res.end(200);
});
```
**Correct (validate against dangerous keys):**
**Correct (JavaScript - validate against dangerous keys):**
```javascript
app.post('/testOk/:id', (req, res) => {
app.post('/test/:id', (req, res) => {
let id = req.params.id;
if (id !== 'constructor' && id !== '__proto__') {
let items = req.session.todos[id];
if (!items) {
items = req.session.todos[id] = {};
}
// ok: prototype-pollution-assignment
items[req.query.name] = req.query.text;
}
res.end(200);
});
```
**Correct (use static keys):**
**Incorrect (JavaScript - nested property assignment in loop):**
```javascript
function ok1(req, res) {
let items = req.session.todos["id"];
if (!items) {
items = req.session.todos["id"] = {};
}
// ok: prototype-pollution-assignment
items[req.query.name] = req.query.text;
res.end(200);
}
function ok2(req, res) {
let id = req.params.id;
let items = req.session.todos[id];
if (!items) {
items = req.session.todos[id] = {};
}
// ok: prototype-pollution-assignment
items["name"] = req.query.text;
res.end(200);
}
```
**Incorrect (prototype pollution in loops):**
```javascript
function test1(name, value) {
if (name.indexOf('.') === -1) {
this.config[name] = value;
return this;
}
let config = this.config;
name = name.split('.');
const length = name.length;
name.forEach((item, index) => {
if (index === length - 1) {
config[item] = value;
} else {
if (!helper.isObject(config[item])) {
config[item] = {};
}
// ruleid:prototype-pollution-loop
config = config[item];
}
});
return this;
}
function test2(obj, props, value) {
if (typeof props == 'string') {
props = props.split('.');
}
if (typeof props == 'symbol') {
props = [props];
}
function setNestedValue(obj, props, value) {
props = props.split('.');
var lastProp = props.pop();
if (!lastProp) {
return false;
}
var thisProp;
while ((thisProp = props.shift())) {
if (typeof obj[thisProp] == 'undefined') {
obj[thisProp] = {};
}
// ruleid:prototype-pollution-loop
obj = obj[thisProp];
if (!obj || typeof obj != 'object') {
return false;
}
}
obj[lastProp] = value;
return true;
}
function test3(obj, prop, val) {
const segs = split(prop);
const last = segs.pop();
while (segs.length) {
const key = segs.shift();
// ruleid:prototype-pollution-loop
obj = obj[key] || (obj[key] = {});
}
obj[last] = val;
}
```
**Correct (use numeric index in loops):**
**Correct (JavaScript - use numeric index or Map):**
```javascript
function okTest1(name) {
if (name.indexOf('.') === -1) {
this.config[name] = value;
return this;
}
function safeIteration(name) {
let config = this.config;
name = name.split('.');
const length = name.length;
name.forEach((item, index) => {
// ok:prototype-pollution-loop
config = config[index];
});
return this;
}
function okTest2(name) {
let config = this.config;
name = name.split('.');
const length = name.length;
for (let i = 0; i < name.length; i++) {
// ok:prototype-pollution-loop
config = config[i];
}
return this;
}
```
**Incorrect (mass assignment via Object.assign in Express):**
**Incorrect (JavaScript - Object.assign with user input):**
```javascript
const express = require('express')
const app = express()
const port = 3000
function testController1(req, res) {
try {
const defaultData = {foo: true}
// ruleid: express-data-exfiltration
let data = Object.assign(defaultData, req.query)
doSmthWith(data)
} catch (err) {
this.log.error(err);
}
res.end('ok')
};
app.get('/test1', testController1)
let testController2 = function (req, res) {
const defaultData = {foo: {bar: true}}
// ruleid: express-data-exfiltration
let data = Object.assign(defaultData, {foo: req.query})
doSmthWith(data)
return res.send({ok: true})
}
app.get('/test2', testController2)
var testController3 = null;
testController3 = function (req, res) {
function controller(req, res) {
const defaultData = {foo: true}
let newData = req.body
// ruleid: express-data-exfiltration
let data = Object.assign(defaultData, newData)
let data = Object.assign(defaultData, req.body)
doSmthWith(data)
return res.send({ok: true})
}
app.get('/test3', testController3)
```
**Correct (use safe data sources in Object.assign):**
**Correct (JavaScript - use trusted data sources):**
```javascript
let okController = function (req, res) {
function controller(req, res) {
const defaultData = {foo: {bar: true}}
// ok: express-data-exfiltration
let data = Object.assign(defaultData, {foo: getFoo()})
let data = Object.assign(defaultData, {foo: getTrustedFoo()})
doSmthWith(data)
return res.send({ok: true})
}
app.get('/ok-test2', okController)
```
---
### Language: Ruby (Rails)
**Incorrect (permitting dangerous attributes):**
```ruby
params = ActionController::Parameters.new({
person: {
name: "Francesco",
age: 22,
role: "admin"
}
})
#ruleid: check-permit-attributes-high
params.permit(:admin)
# ruleid: check-permit-attributes-medium
params.permit(:role_id)
```
**Correct (permit only safe attributes):**
```ruby
#ok: check-permit-attributes-high
params.permit(:some_safe_property)
#ok: check-permit-attributes-medium
params.permit(:some_safe_property)
```
**Incorrect (dangerous attr_accessible and permit usage):**
```ruby
class Bad_attr_accessible
include ActiveModel::MassAssignmentSecurity
# ruleid: model-attr-accessible
attr_accessible :name, :admin,
:telephone, as: :create_params
# ruleid: model-attr-accessible
attr_accessible :name, :banned,
as: :create_params
# ruleid: model-attr-accessible
attr_accessible :role,
:telephone, as: :create_params
# ruleid: model-attr-accessible
attr_accessible :name,
:account_id, as: :create_params
# ruleid: model-attr-accessible
User.new(params.permit(:name, :admin))
# ruleid: model-attr-accessible
params_with_conditional_require(ctrl.params).permit(:name, :age, :admin)
# ruleid: model-attr-accessible
User.new(params.permit(:role))
# ruleid: model-attr-accessible
User.new(params.permit(:banned, :name))
# ruleid: model-attr-accessible
User.new(params.permit(:address, :account_id, :age))
# ruleid: model-attr-accessible
params.permit!
end
```
**Correct (safe attr_accessible and permit usage):**
```ruby
class Ok_attr_accessible
# ok: model-attr-accessible
attr_accessible :name, :address, :age,
:telephone, as: :create_params
# ok: model-attr-accessible
User.new(params.permit(:address, :acc, :age))
# ok: model-attr-accessible
params_with_conditional_require(ctrl.params).permit(:name, :address, :age)
end
```
**Incorrect (create_with bypasses strong parameters):**
```ruby
def bad_create_with
# ruleid: create-with
user.blog_posts.create_with(params[:blog_post]).create
end
```
**Correct (use permit with create_with):**
```ruby
def create
# ok: create-with
user.blog_posts.create(params[:blog_post])
# ok: create-with
user.blog_posts.create_with(params[:blog_post].permit(:title, :body, :etc)).create
end
```
**Incorrect (mass assignment without attr_accessible):**
```ruby
def mass_assign_unsafe
#ruleid: mass-assignment-vuln
User.new(params[:user])
#ruleid: mass-assignment-vuln
user = User.new(params[:user])
#ruleid: mass-assignment-vuln
User.new(params[:user], :without_protection => true)
end
```
**Correct (use attr_accessible before mass assignment):**
```ruby
def safe_send
#ok: mass-assignment-vuln
attr_accessible :name
User.new(params[:user])
#ok: mass-assignment-vuln
attr_accessible :name
user = User.new(params[:user])
end
```
**Incorrect (disabling mass assignment protection):**
```ruby
# ruleid:mass-assignment-protection-disabled
User.new(params[:user], :without_protection => true)
```
**Correct (do not disable protection):**
```ruby
# ok:mass-assignment-protection-disabled
User.new(params[:user])
```
**Incorrect (model without attr_accessible):**
```ruby
# ruleid: model-attributes-attr-accessible
class User < ActiveRecord::Base
acts_as_authentic do |t|
t.login_field=:login # for available options see documentation in: Authlogic::ActsAsAuthentic
end # block optional
has_attached_file :avatar, :styles => { :medium => "300x300>", :thumb => "100x100>" }
end
def create
user = User.create(person_params)
end
```
**Correct (model with attr_accessible):**
```ruby
class User < ActiveRecord::Base
acts_as_authentic do |t|
t.login_field=:login # for available options see documentation in: Authlogic::ActsAsAuthentic
end # block optional
attr_accessible :login
attr_accessible :first_name
attr_accessible :middle_name
attr_accessible :surname
attr_accessible :permanent_address
attr_accessible :correspondence_address
attr_accessible :email
attr_accessible :contact_no
attr_accessible :gender
attr_accessible :password
attr_accessible :password_confirmation
attr_accessible :avatar
has_attached_file :avatar, :styles => { :medium => "300x300>", :thumb => "100x100>" }
end
def create
user = User.create(person_params)
end
```
---
### Language: Python (Django)
**Incorrect (mass assignment using **request):**
```python
from django.shortcuts import render
from myapp.models import Whatzit
# Test cases borrowed from https://gist.github.com/jsocol/3217262
def create_whatzit(request):
# ruleid: mass-assignment
Whatzit.objects.create(**request.POST)
return render(request, 'created.html')
def update_whatzit(request, id):
whatzit = Whatzit.objects.filter(pk=id)
# ruleid: mass-assignment
whatzit.update(**request.POST)
whatzit.save()
return render(request, 'saved.html')
```
**Correct (explicitly assign each field):**
```python
def good_whatzit(request):
# ok: mass-assignment
Whatzit.objects.create(
name=request.POST.get('name'),
dob=request.POST.get('dob')
)
return render(request, 'created.html')
```
---
### Language: PHP (Laravel)
**Incorrect (empty $guarded allows mass assignment):**
```php
<?php
namespace App\Models;
use Illuminate\Database\Eloquent\Model;
class Flight extends Model
{
/**
* The primary key associated with the table.
*
* @var string
*/
protected $primaryKey = 'flight_id';
/**
* The attributes that aren't mass assignable.
*
* @var array
*/
// ruleid: laravel-dangerous-model-construction
protected $guarded = [];
}
```
**Correct (use $fillable to explicitly allowlist attributes):**
```php
<?php
namespace App\Models;
use Illuminate\Database\Eloquent\Model;
class Flight extends Model
{
/**
* The attributes that are mass assignable.
*
* @var array
*/
protected $fillable = ['name', 'destination'];
}
```
---
### Language: C# (.NET)
**Incorrect (model binding without [Bind] attribute):**
```csharp
using Microsoft.AspNetCore.Mvc;
public IActionResult Create(UserModel model)
{
context.SaveChanges();
// ruleid: mass-assignment
return View("Index", model);
}
```
**Correct (use [Bind] attribute to allowlist properties):**
```csharp
using Microsoft.AspNetCore.Mvc;
public IActionResult Create([Bind(nameof(UserModel.Name))] UserModel model)
{
context.SaveChanges();
// ok: mass-assignment
return View("Index", model);
}
[HttpGet("/")]
public IActionResult Index()
{
// ok: mass-assignment
return NoContent();
}
```
---
**References:**
- CWE-915: Improperly Controlled Modification of Dynamically-Determined Object Attributes
- [OWASP Mass Assignment Cheat Sheet](https://cheatsheetseries.owasp.org/cheatsheets/Mass_Assignment_Cheat_Sheet.html)
- [OWASP Top 10 A08:2021 - Software and Data Integrity Failures](https://owasp.org/Top10/A08_2021-Software_and_Data_Integrity_Failures/)
- [JavaScript Prototype Pollution Attack in NodeJS (PDF)](https://github.com/HoLyVieR/prototype-pollution-nsec18/blob/master/paper/JavaScript_prototype_pollution_attack_in_NodeJS.pdf)
- [Laravel Mass Assignment Documentation](https://laravel.com/docs/9.x/eloquent#allowing-mass-assignment)
- [OWASP API Security - Mass Assignment](https://github.com/OWASP/API-Security/blob/master/2019/en/src/0xa6-mass-assignment.md)
- [Brakeman Mass Assignment Checks](https://github.com/presidentbeef/brakeman/blob/main/lib/brakeman/checks/check_model_attr_accessible.rb)
skills/code-security/rules/regex-dos.md
+44 -562
View File
@@ -18,204 +18,60 @@ Common vulnerable patterns include:
**Incorrect (vulnerable ReDoS pattern):**
```javascript
// ruleid: detect-redos
const re = new RegExp("([a-z]+)+$", "i");
// ruleid: detect-redos
const re = new RegExp(/([a-z]+)+$/, "i");
var r = /^\w+([-_+.]\w+)*@\w+([-.]\w+)*\.\w+([-.]\w+)*$/
// ruleid: detect-redos
new RegExp(r, "i");
// ruleid: detect-redos
r.test(a)
// ruleid: detect-redos
"a".match(r)
var emailRegex = /^\w+([-_+.]\w+)*@\w+([-.]\w+)*\.\w+([-.]\w+)*$/;
emailRegex.test(userInput);
```
**Correct (safe regex patterns):**
```javascript
// ok: detect-redos
"a".match(b)
// ok: detect-redos
"a".match("([a-z])")
var c = /([a-z])/
// ok: detect-redos
c.test(a)
```
// Use atomic patterns without nested quantifiers
const safeRegex = /^[a-z]+$/i;
**References:**
- [OWASP ReDoS](https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS)
- [Regular-Expressions.info ReDoS](https://www.regular-expressions.info/redos.html)
- CWE-1333: Inefficient Regular Expression Complexity
// Or use a library with ReDoS protection
import { RE2 } from 're2';
const re = new RE2("([a-z]+)+$");
```
---
**Incorrect (non-literal RegExp with user input):**
```javascript
function bad (name) {
//ruleid: detect-non-literal-regexp
const reg = new RegExp("\\w+" + name)
return reg.exec(name)
function searchHandler(userPattern) {
const reg = new RegExp("\\w+" + userPattern);
return reg.exec(data);
}
```
**Correct (hardcoded regex patterns):**
```javascript
function ok (name) {
//ok: detect-non-literal-regexp
const reg = new RegExp("\\w+")
return reg.exec(name)
}
function jsliteral (name) {
const exp = /a.*/;
//ok: detect-non-literal-regexp
const reg = new RegExp(exp);
return reg.exec(name);
function searchHandler(userInput) {
const reg = new RegExp("\\w+");
return reg.exec(userInput);
}
```
**References:**
- [OWASP ReDoS](https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS)
- CWE-1333: Inefficient Regular Expression Complexity
---
**Incorrect (incomplete string sanitization):**
```javascript
function escapeQuotes(s) {
// ruleid:incomplete-sanitization
return s.replace("'", "''");
}
function removeTabs(s) {
// ruleid:incomplete-sanitization
return s.replace('\t', "");
}
function escapeHtml(html) {
// ruleid:incomplete-sanitization
return html
.replace("<", "")
.replace(">", "");
return s.replace("'", "''"); // Only replaces first occurrence
}
```
**Correct (use regex with global flag):**
```javascript
function okTest(s) {
return s.replace("foo", "bar");
}
function okEscapeQuotes(s) {
return s.replace(/'/g, "''");
function escapeQuotes(s) {
return s.replace(/'/g, "''"); // Replaces all occurrences
}
```
**References:**
- [OWASP Injection](https://owasp.org/Top10/A03_2021-Injection)
- CWE-116: Improper Encoding or Escaping of Output
---
**Incorrect (Ajv allErrors: true enables DoS):**
```javascript
import express from 'express';
import Ajv from 'ajv';
function test1() {
const settings = { allErrors: true, smth: 'else' }
// ruleid: ajv-allerrors-true
const ajv1 = new Ajv(settings);
return ajv1
}
function test2() {
// ruleid: ajv-allerrors-true
var ajv = new Ajv({ allErrors: true, smth: 'else' });
ajv.addSchema(schema, 'input');
}
function test3() {
// ruleid: ajv-allerrors-true
var ajv = new Ajv({ smth: 'else', allErrors: true });
ajv.addSchema(schema, 'input');
}
function test4() {
// ruleid: ajv-allerrors-true
var ajv = new Ajv({ smth: 'else', smth: 'else', allErrors: true, smth: 'else' });
ajv.addSchema(schema, 'input');
}
```
**Correct (disable allErrors in production):**
```javascript
function okTest1() {
// ok: ajv-allerrors-true
let ajv = new Ajv({ allErrors: process.env.DEBUG });
ajv.addSchema(schema, 'input');
}
function okTest2() {
// ok: ajv-allerrors-true
var ajv = new Ajv({ smth: 'else', allErrors: false });
ajv.addSchema(schema, 'input');
}
```
**References:**
- [Ajv allErrors Option](https://ajv.js.org/options.html#allerrors)
- CWE-400: Uncontrolled Resource Consumption
---
### Language: TypeScript
**Incorrect (CORS regex with unescaped dots):**
```typescript
const corsDomains = [
/localhost\:/,
/(.+\.)*foo\.com$/,
/(.+\.)*foobar\.com$/, // matches *.foobar.com,
// ruleid: cors-regex-wildcard
/^(http|https):\/\/(qix|qux).biz.baz.foobar.com$/,
/^(http|https):\/\/www\.bar\.com$/,
// ruleid: cors-regex-wildcard
/^(http|https):\/\/www.foo.com$/,
];
const CORS = [
/localhost\:/,
/(.+\.)*foo\.com$/,
/(.+\.)*foobar\.com$/, // matches *.foobar.com,
// ruleid: cors-regex-wildcard
/^(http|https):\/\/(qix|qux).biz.baz.foobar.com$/,
/^(http|https):\/\/www\.bar\.com$/,
// ruleid: cors-regex-wildcard
/^(http|https):\/\/www.foo.com$/,
];
// ruleid: cors-regex-wildcard
const corsOrigin = /^(http|https):\/\/www.foo.com$/;
```
**Correct (escape dots in CORS regex):**
```typescript
const urls = [
/localhost\:/,
/(.+\.)*foo\.com$/,
/(.+\.)*foobar\.com$/, // matches *.foobar.com,
/^(http|https):\/\/(qix|qux).biz.baz.foobar.com$/,
/^(http|https):\/\/www\.bar\.com$/,
/^(http|https):\/\/www.foo.com$/,
];
```
**References:**
- [OWASP Insecure Design](https://owasp.org/Top10/A04_2021-Insecure_Design)
- CWE-183: Permissive List of Allowed Inputs
- [OWASP ReDoS](https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS)
- [Regular-Expressions.info ReDoS](https://www.regular-expressions.info/redos.html)
- CWE-1333: Inefficient Regular Expression Complexity
---
@@ -225,51 +81,33 @@ const urls = [
```python
import re
redos = r"^(a+)+$"
regex = r"^[0-9]+$"
redos_pattern = r"^(a+)+$"
data = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaX"
data = "foo"
# ruleid: regex_dos
pattern = re.compile(redos)
pattern.search(data)
# ruleid: regex_dos
pattern = re.compile(redos)
pattern.match(data)
# ruleid: regex_dos
pattern = re.compile(redos)
pattern.findall(data)
pattern = re.compile(redos_pattern)
pattern.match(data) # Catastrophic backtracking
```
**Correct (safe regex patterns):**
```python
import re
redos = r"^(a+)+$"
regex = r"^[0-9]+$"
safe_pattern = r"^a+$"
data = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaX"
data = "foo"
pattern = re.compile(safe_pattern)
pattern.match(data) # Fast failure, no backtracking
```
# ok: regex_dos
pattern = re.compile(regex)
pattern.search(data)
**Mitigation strategies:**
```python
# Use regex timeout (Python 3.11+)
import re
re.match(pattern, data, timeout=1.0)
# ok: regex_dos
pattern = re.compile(regex)
pattern.fullmatch(data)
# ok: regex_dos
pattern = re.compile(regex)
pattern.split(data)
# ok: regex_dos
pattern.escape(redos)
# ok: regex_dos
pattern = re.compile(redos)
pattern.purge()
# Or use google-re2 library for linear-time matching
import re2
re2.match(r"^(a+)+$", data)
```
**References:**
@@ -278,373 +116,17 @@ pattern.purge()
---
**Incorrect (missing Django REST Framework throttle config):**
```python
# ruleid: missing-throttle-config
REST_FRAMEWORK = {
'PAGE_SIZE': 10
}
```
## General Mitigation Strategies
**Correct (throttle config enabled):**
```python
# ok: missing-throttle-config
REST_FRAMEWORK = {
'PAGE_SIZE': 10,
'DEFAULT_THROTTLE_CLASSES': [
'rest_framework.throttling.AnonRateThrottle',
'rest_framework.throttling.UserRateThrottle'
],
'DEFAULT_THROTTLE_RATES': {
'anon': '100/day',
'user': '1000/day'
},
"SOMETHING_ELSE": {1: 2}
}
```
**References:**
- [Django REST Framework Throttling](https://www.django-rest-framework.org/api-guide/throttling/#setting-the-throttling-policy)
- CWE-400: Uncontrolled Resource Consumption
---
### Language: Ruby
**Incorrect (user-controlled regex):**
```ruby
def some_rails_controller
foo = params[:some_regex]
#ruleid: check-regex-dos
Regexp.new(foo).match("some_string")
end
def some_rails_controller
foo = Record[something]
#ruleid: check-regex-dos
Regexp.new(foo).match("some_string")
end
def some_rails_controller
foo = Record.read_attribute("some_attribute")
#ruleid: check-regex-dos
Regexp.new(foo).match("some_string")
end
def use_params_in_regex
#ruleid: check-regex-dos
@x = something.match /#{params[:x]}/
end
```
**Correct (safe regex usage):**
```ruby
def some_rails_controller
bar = ENV['someEnvVar']
#ok: check-regex-dos
Regexp.new(bar).match("some_string")
end
def regex_on_params
#ok: check-regex-dos
@x = params[:x].match /foo/
end
```
**References:**
- [OWASP ReDoS](https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS)
- CWE-1333: Inefficient Regular Expression Complexity
---
**Incorrect (incorrectly-bounded Rails validation regex):**
```ruby
class Account < ActiveRecord::Base
#ruleid: check-validation-regex
validates :username, :length => 6..20, :format => /([a-z][0-9])+/i
#ruleid: check-validation-regex
validates :phone, :format => { :with => /(\d{3})-(\d{3})-(\d{4})/, :on => :create }, :presence => true
#ruleid: check-validation-regex
validates :first_name, :format => /\w+/
serialize :cc_info #safe from CVE-2013-0277
attr_accessible :blah_admin_blah
end
class Account < ActiveRecord::Base
#ruleid: check-validation-regex
validates_format_of :name, :with => /^[a-zA-Z]+$/
#ruleid: check-validation-regex
validates_format_of :blah, :with => /\A[a-zA-Z]+$/
#ruleid: check-validation-regex
validates_format_of :blah2, :with => /^[a-zA-Z]+\Z/
#ruleid: check-validation-regex
validates_format_of :something, :with => /[a-zA-Z]\z/
end
```
**Correct (properly-bounded regex with \A and \Z):**
```ruby
class Account < ActiveRecord::Base
#ok: check-validation-regex
validates_format_of :good_valid, :with => /\A[a-zA-Z]\z/ #No warning
#ok: check-validation-regex
validates_format_of :not_bad, :with => /\A[a-zA-Z]\Z/ #No warning
end
```
Ruby regex behavior is multiline by default. Use `\A` for beginning of string and `\Z` (or `\z`) for end of string instead of `^` and `$`.
**References:**
- [Brakeman Format Validation](https://brakemanscanner.org/docs/warning_types/format_validation/)
- CWE-185: Incorrect Regular Expression
---
### Language: C#
**Incorrect (regex without timeout on untrusted input):**
```csharp
using System.Text.RegularExpressions;
namespace RegularExpressionsDos
{
public class RegularExpressionsDos
{
// ruleid: regular-expression-dos
public void ValidateRegex(string search)
{
Regex rgx = new Regex("^A(B|C+)+D");
rgx.Match(search);
}
// ruleid: regular-expression-dos
public void ValidateRegex2(string search)
{
Regex rgx = new Regex("^A(B|C+)+D", new RegexOptions { });
rgx.Match(search);
}
// ruleid: regular-expression-dos
public void Validate4(string search)
{
var pattern = @"^A(B|C+)+D";
var result = Regex.Match(search, pattern);
}
// ruleid: regular-expression-dos
public void Validate5(string search)
{
var pattern = @"^A(B|C+)+D";
var result = Regex.Match(search, pattern, new RegexOptions { });
}
}
}
```
**Correct (regex with timeout):**
```csharp
using System.Text.RegularExpressions;
namespace RegularExpressionsDos
{
public class RegularExpressionsDos
{
// ok: regular-expression-dos
public void ValidateRegex3(string search)
{
Regex rgx = new Regex("^A(B|C+)+D", new RegexOptions { }, TimeSpan.FromSeconds(2000));
rgx.Match(search);
}
// ok: regular-expression-dos
public void Validate5(string search)
{
var pattern = @"^A(B|C+)+D";
var result = Regex.Match(search, pattern, new RegexOptions { }, TimeSpan.FromSeconds(2000));
}
}
}
```
**References:**
- [OWASP ReDoS](https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS)
- [.NET Regular Expressions](https://docs.microsoft.com/en-us/dotnet/standard/base-types/regular-expressions#regular-expression-examples)
- CWE-1333: Inefficient Regular Expression Complexity
---
**Incorrect (regex with excessive or infinite timeout):**
```csharp
using System.Text.RegularExpressions;
namespace RegularExpressionsDosInfiniteTimeout
{
public class RegularExpressionsDosInfiniteTimeout
{
// ruleid: regular-expression-dos-infinite-timeout
Regex rgx = new Regex(pattern, RegexOptions.IgnoreCase, TimeSpan.FromSeconds(10));
// ruleid: regular-expression-dos-infinite-timeout
Regex rgx = new Regex(pattern, RegexOptions.IgnoreCase, TimeSpan.InfiniteMatchTimeout);
// ruleid: regular-expression-dos-infinite-timeout
Regex rgx = new Regex(pattern, RegexOptions.IgnoreCase, TimeSpan.FromMinutes(1));
// ruleid: regular-expression-dos-infinite-timeout
Regex rgx = new Regex(pattern, RegexOptions.IgnoreCase, TimeSpan.FromHours(1));
}
}
```
**Correct (regex with short timeout):**
```csharp
using System.Text.RegularExpressions;
namespace RegularExpressionsDosInfiniteTimeout
{
public class RegularExpressionsDosInfiniteTimeout
{
// ok
Regex rgx = new Regex(pattern, RegexOptions.IgnoreCase, TimeSpan.FromSeconds(1));
}
}
```
Consider setting the timeout to a short amount of time like 2 or 3 seconds. If you are sure you need an infinite timeout, double check that your context meets the conditions outlined in the "Notes to Callers" section at the bottom of this page: https://docs.microsoft.com/en-us/dotnet/api/system.text.regularexpressions.regex.-ctor?view=net-6.0
**References:**
- [OWASP ReDoS](https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS)
- [Regex.InfiniteMatchTimeout](https://docs.microsoft.com/en-us/dotnet/api/system.text.regularexpressions.regex.infinitematchtimeout)
- [Regex Constructor](https://docs.microsoft.com/en-us/dotnet/api/system.text.regularexpressions.regex.-ctor?view=net-6.0)
- CWE-1333: Inefficient Regular Expression Complexity
---
### Language: Go
**Incorrect (decompression without size limit - zip bomb):**
```go
// cf. https://github.com/securego/gosec/blob/master/testutils/source.go#L684
package main
import (
"bytes"
"compress/zlib"
"io"
"os"
)
func blah() {
buff := []byte{120, 156, 202, 72, 205, 201, 201, 215, 81, 40, 207,
47, 202, 73, 225, 2, 4, 0, 0, 255, 255, 33, 231, 4, 147}
b := bytes.NewReader(buff)
r, err := zlib.NewReader(b)
if err != nil {
panic(err)
}
// ruleid: potential-dos-via-decompression-bomb
_, err := io.Copy(os.Stdout, r)
if err != nil {
panic(err)
}
r.Close()
}
func blah2() {
buff := []byte{120, 156, 202, 72, 205, 201, 201, 215, 81, 40, 207,
47, 202, 73, 225, 2, 4, 0, 0, 255, 255, 33, 231, 4, 147}
b := bytes.NewReader(buff)
r, err := zlib.NewReader(b)
if err != nil {
panic(err)
}
buf := make([]byte, 8)
// ruleid: potential-dos-via-decompression-bomb
_, err := io.CopyBuffer(os.Stdout, r, buf)
if err != nil {
panic(err)
}
r.Close()
}
func blah3() {
r, err := zip.OpenReader("tmp.zip")
if err != nil {
panic(err)
}
defer r.Close()
for i, f := range r.File {
out, err := os.OpenFile("output" + strconv.Itoa(i), os.O_WRONLY|os.O_CREATE|os.O_TRUNC, f.Mode())
if err != nil {
panic(err)
}
rc, err := f.Open()
if err != nil {
panic(err)
}
// ruleid: potential-dos-via-decompression-bomb
_, err = io.Copy(out, rc)
out.Close()
rc.Close()
if err != nil {
panic(err)
}
}
}
```
**Correct (use io.CopyN with size limit):**
```go
func benign() {
s, err := os.Open("src")
if err != nil {
panic(err)
}
defer s.Close()
d, err := os.Create("dst")
if err != nil {
panic(err)
}
defer d.Close()
// ok: potential-dos-via-decompression-bomb
_, err = io.Copy(d, s)
if err != nil {
panic(err)
}
}
func ok() {
buff := []byte{120, 156, 202, 72, 205, 201, 201, 215, 81, 40, 207,
47, 202, 73, 225, 2, 4, 0, 0, 255, 255, 33, 231, 4, 147}
b := bytes.NewReader(buff)
r, err := zlib.NewReader(b)
if err != nil {
panic(err)
}
buf := make([]byte, 8)
// ok: potential-dos-via-decompression-bomb
_, err := io.CopyN(os.Stdout, r, buf, 1024*1024*4)
if err != nil {
panic(err)
}
r.Close()
}
```
By limiting the max bytes read with `io.CopyN()`, you can mitigate zip bomb attacks.
**References:**
- [Go io.CopyN](https://golang.org/pkg/io/#CopyN)
- [gosec decompression-bomb rule](https://github.com/securego/gosec/blob/master/rules/decompression-bomb.go)
- CWE-400: Uncontrolled Resource Consumption
---
1. **Avoid nested quantifiers**: Never use patterns like `(a+)+` or `(.*)*`
2. **Use atomic groups or possessive quantifiers** when available
3. **Set timeouts**: Use regex timeout mechanisms to limit execution time
4. **Use safe regex libraries**: RE2 (Go/Python/JS) guarantees linear-time matching
5. **Validate user input length**: Limit input size before regex matching
6. **Test with ReDoS analyzers**: Use tools like `safe-regex` or `recheck`
**References:**
- CWE-1333: Inefficient Regular Expression Complexity
- CWE-400: Uncontrolled Resource Consumption
- CWE-185: Incorrect Regular Expression
- [OWASP ReDoS](https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS)
- [Regular-Expressions.info ReDoS](https://www.regular-expressions.info/redos.html)
skills/code-security/rules/secrets.md
+52 -568
View File
@@ -7,144 +7,63 @@ impact: CRITICAL
Hardcoded credentials, API keys, tokens, and other secrets in source code pose a critical security risk. When secrets are committed to version control, they can be exposed to unauthorized parties through repository access, leaked in public repositories or through data breaches, difficult to rotate without code changes and redeployment, and discovered by automated secret scanning tools used by attackers. Always use environment variables, secret managers, or secure vaults to provide credentials at runtime.
**Incorrect (Python - hardcoded AWS credentials with boto3):**
### AWS Credentials
**Incorrect (Python - hardcoded AWS credentials):**
```python
import boto3
from boto3 import client
# ruleid:hardcoded-token
client("s3", aws_secret_access_key="jWnyxxxxxxxxxxxxxxxxX7ZQxxxxxxxxxxxxxxxx")
# ruleid:hardcoded-token
boto3.sessions.Session(aws_secret_access_key="jWnyxxxxxxxxxxxxxxxxX7ZQxxxxxxxxxxxxxxxx")
s = boto3.sessions
# ruleid:hardcoded-token
s.Session(aws_access_key_id="AKIAxxxxxxxxxxxxxxxx")
uhoh_key = "AKIAxxxxxxxxxxxxxxxx"
ok_secret = os.environ.get("SECRET_ACCESS_KEY")
# ruleid:hardcoded-token
s3 = boto3.resource(
"s3",
aws_access_key_id=uhoh_key,
aws_secret_access_key=ok_secret,
region_name="sfo2",
endpoint_url="https://sfo2.digitaloceanspaces.com",
)
ok_key = os.environ.get("ACCESS_KEY_ID")
uhoh_secret = "jWnyxxxxxxxxxxxxxxxxX7ZQxxxxxxxxxxxxxxxx"
# ruleid:hardcoded-token
s3 = boto3.resource(
"s3",
aws_access_key_id=ok_key,
aws_secret_access_key=uhoh_secret,
region_name="sfo2",
endpoint_url="https://sfo2.digitaloceanspaces.com",
aws_access_key_id="AKIAxxxxxxxxxxxxxxxx",
aws_secret_access_key="jWnyxxxxxxxxxxxxxxxxX7ZQxxxxxxxxxxxxxxxx",
region_name="us-east-1",
)
```
**Correct (Python - AWS credentials from environment variables):**
**Correct (Python - AWS credentials from environment):**
```python
import boto3
import os
# ok:hardcoded-token
key = os.environ.get("ACCESS_KEY_ID")
secret = os.environ.get("SECRET_ACCESS_KEY")
s3 = boto3.resource(
"s3",
aws_access_key_id=key,
aws_secret_access_key=secret,
region_name="sfo2",
endpoint_url="https://sfo2.digitaloceanspaces.com",
)
# ok:hardcoded-token
s3 = client("s3", aws_access_key_id="this-is-not-a-key")
# ok:hardcoded-token - placeholder values
s3 = boto3.resource(
"s3",
aws_access_key_id="<your token here>",
aws_secret_access_key="<your secret here>",
region_name="us-east-1",
)
```
**Incorrect (Go - hardcoded AWS access token pattern):**
### API Keys and Tokens
```go
// ruleid: aws-access-token
AWS_api_token = "AKIALALEMEL33243OLIB"
```
**Correct (Go - AWS token from environment):**
```go
// ok: aws-access-token
AWS_api_token = os.Getenv("AWS_ACCESS_KEY_ID")
```
**Incorrect (JavaScript - hardcoded JWT secret with jsonwebtoken):**
**Incorrect (JavaScript - hardcoded JWT secret):**
```javascript
"use strict";
const config = require('./config')
const jsonwt = require('jsonwebtoken')
function example1() {
function signToken() {
const payload = {foo: 'bar'}
const secret = 'shhhhh'
// ruleid: hardcoded-jwt-secret
const token1 = jsonwt.sign(payload, secret)
}
function example2() {
const payload = {foo: 'bar'}
// ruleid: hardcoded-jwt-secret
const token2 = jsonwt.sign(payload, 'some-secret')
}
const Promise = require("bluebird");
const secret = "hardcoded-secret"
class Authentication {
static sign(obj){
// ruleid: hardcoded-jwt-secret
return jsonwt.sign(obj, secret, {});
}
const token = jsonwt.sign(payload, 'my-secret-key')
return token
}
```
**Correct (JavaScript - JWT secret from config or environment):**
**Correct (JavaScript - JWT secret from environment):**
```javascript
const config = require('./config')
const jsonwt = require('jsonwebtoken')
function example3() {
// ok: hardcoded-jwt-secret
function signToken() {
const payload = {foo: 'bar'}
const token3 = jsonwt.sign(payload, config.secret)
}
function example4() {
// ok: hardcoded-jwt-secret
const payload = {foo: 'bar'}
const secret2 = config.secret
const token4 = jsonwt.sign(payload, secret2)
}
function example5() {
// ok: hardcoded-jwt-secret
const payload = {foo: 'bar'}
const secret3 = process.env.SECRET
const token5 = jsonwt.sign(payload, secret3)
const secret = process.env.JWT_SECRET
const token = jsonwt.sign(payload, secret)
return token
}
```
@@ -153,344 +72,31 @@ function example5() {
```javascript
var jwt = require('express-jwt');
// ruleid: express-jwt-hardcoded-secret
app.get('/protected', jwt({ secret: 'shhhhhhared-secret' }), function(req, res) {
if (!req.user.admin) return res.sendStatus(401);
res.sendStatus(200);
});
// ruleid: express-jwt-hardcoded-secret
let hardcodedSecret = 'shhhhhhared-secret'
app.get('/protected2', jwt({ secret: hardcodedSecret }), function(req, res) {
if (!req.user.admin) return res.sendStatus(401);
res.sendStatus(200);
});
let secret = "hardcode"
const opts = Object.assign({issuer: 'http://issuer'}, {secret: secret})
app.get('/protected3', jwt(opts), function(req, res) {
if (!req.user.admin) return res.sendStatus(401);
res.sendStatus(200);
});
```
**Correct (JavaScript - express-jwt secret from environment or config):**
**Correct (JavaScript - express-jwt secret from environment):**
```javascript
var jwt = require('express-jwt');
// ok: express-jwt-hardcoded-secret
app.get('/ok-protected', jwt({ secret: process.env.SECRET }), function(req, res) {
if (!req.user.admin) return res.sendStatus(401);
res.sendStatus(200);
});
let configSecret = config.get('secret')
const opts = Object.assign({issuer: 'http://issuer'}, {secret: configSecret})
// ok: express-jwt-hardcoded-secret
app.get('/ok-protected', jwt(opts), function(req, res) {
app.get('/protected', jwt({ secret: process.env.JWT_SECRET }), function(req, res) {
if (!req.user.admin) return res.sendStatus(401);
res.sendStatus(200);
});
```
**Incorrect (TypeScript - hardcoded express-session secret):**
```typescript
import express from 'express'
import session from 'express-session'
const app = express()
let a = 'a'
let config = {
// ruleid: express-session-hardcoded-secret
secret: 'a',
resave: false,
saveUninitialized: false,
}
app.use(session({
// ruleid: express-session-hardcoded-secret
secret: a,
resave: false,
saveUninitialized: false,
}));
app.use(session(config));
let secret2 = {
resave: false,
// ruleid: express-session-hardcoded-secret
secret: 'foo',
saveUninitialized: false,
}
app.use(session(secret2));
```
**Correct (TypeScript - express-session secret from config):**
```typescript
import express from 'express'
import session from 'express-session'
const app = express()
let config1 = {
// ok: express-session-hardcoded-secret
secret: config.secret,
resave: false,
saveUninitialized: false,
}
app.use(session(config1));
app.use(session({
// ok: express-session-hardcoded-secret
secret: config.secret,
resave: false,
saveUninitialized: false,
}));
```
**Incorrect (Go - hardcoded jwt-go key):**
```go
package main
import (
"github.com/dgrijalva/jwt-go"
)
func Signin(w http.ResponseWriter, r *http.Request) {
// Create the JWT key used to create the signature
var jwtKey = []byte("my_secret_key")
// Declare the token with the algorithm used for signing, and the claims
token := jwt.NewWithClaims(jwt.SigningMethodHS256, claims)
// ruleid: hardcoded-jwt-key
tokenString, err := token.SignedString(jwtKey)
// ruleid: hardcoded-jwt-key
tokenString, err := token.SignedString([]byte("my_secret_key"))
if err != nil {
w.WriteHeader(http.StatusInternalServerError)
return
}
}
```
**Correct (Go - JWT key from environment):**
```go
package main
import (
"os"
"github.com/dgrijalva/jwt-go"
)
func Signin(w http.ResponseWriter, r *http.Request) {
// ok: hardcoded-jwt-key - get secret from environment
var jwtKey = []byte(os.Getenv("JWT_SECRET"))
token := jwt.NewWithClaims(jwt.SigningMethodHS256, claims)
tokenString, err := token.SignedString(jwtKey)
if err != nil {
w.WriteHeader(http.StatusInternalServerError)
return
}
}
```
**Incorrect (Java - hardcoded java-jwt secret):**
```java
package jwt_test.jwt_test_1;
import com.auth0.jwt.JWT;
import com.auth0.jwt.algorithms.Algorithm;
import com.auth0.jwt.exceptions.JWTCreationException;
public class App
{
static String secret = "secret";
private static void bad1() {
try {
// ruleid: java-jwt-hardcoded-secret
Algorithm algorithm = Algorithm.HMAC256("secret");
String token = JWT.create()
.withIssuer("auth0")
.sign(algorithm);
} catch (JWTCreationException exception){
//Invalid Signing configuration / Couldn't convert Claims.
}
}
}
abstract class App2
{
// ruleid: java-jwt-hardcoded-secret
static String secret = "secret";
public void bad2() {
try {
Algorithm algorithm = Algorithm.HMAC256(secret);
String token = JWT.create()
.withIssuer("auth0")
.sign(algorithm);
} catch (JWTCreationException exception){
//Invalid Signing configuration / Couldn't convert Claims.
}
}
}
```
**Correct (Java - JWT secret from parameter):**
```java
package jwt_test.jwt_test_1;
import com.auth0.jwt.JWT;
import com.auth0.jwt.algorithms.Algorithm;
import com.auth0.jwt.exceptions.JWTCreationException;
public class App
{
private static void ok1(String secretKey) {
try {
// ok: java-jwt-hardcoded-secret
Algorithm algorithm = Algorithm.HMAC256(secretKey);
String token = JWT.create()
.withIssuer("auth0")
.sign(algorithm);
} catch (JWTCreationException exception){
//Invalid Signing configuration / Couldn't convert Claims.
}
}
public static void main( String[] args )
{
ok1(args[0]);
}
}
```
**Incorrect (Go - hardcoded GitHub personal access token):**
```go
// ruleid: github-pat
github_api_token = "ghp_emmtytndiqky5a98w0s98w36vfhiz6f7ed4c"
```
**Correct (Go - GitHub token from environment):**
```go
// ok: github-pat
github_api_token = os.Getenv("GITHUB_TOKEN")
```
**Incorrect (Go - hardcoded Stripe access token):**
```go
// ruleid: stripe-access-token
stripeToken := "sk_test_20cbqx6v2hpftsbq203r36yqccazez"
```
**Correct (Go - Stripe token from environment):**
```go
// ok: stripe-access-token
stripeToken := os.Getenv("STRIPE_SECRET_KEY")
```
**Incorrect (Go - hardcoded private key):**
```go
// ruleid: private-key
[]string{`-----BEGIN PRIVATE KEY-----
anything
-----END PRIVATE KEY-----`,
`-----BEGIN RSA PRIVATE KEY-----
abcdefghijklmnopqrstuvwxyz
-----END RSA PRIVATE KEY-----
`,
`-----BEGIN PRIVATE KEY BLOCK-----
anything
-----END PRIVATE KEY BLOCK-----`,
}
```
**Correct (Go - private key from file or environment):**
```go
// ok: private-key - load from file or environment
privateKey, err := ioutil.ReadFile(os.Getenv("PRIVATE_KEY_PATH"))
if err != nil {
log.Fatal(err)
}
```
**Incorrect (Ruby - hardcoded secrets):**
```ruby
# ruleid: check-secrets
PASSWORD = "superdupersecret"
http_basic_authenticate_with :name => "superduperadmin", :password => PASSWORD, :only => :create
```
**Correct (Ruby - secrets from secure store):**
```ruby
# ok: check-secrets
secret = get_from_store('somepass')
# ok: check-secrets
rest_auth_site_key = ""
```
**Incorrect (Ruby - hardcoded HTTP auth password in controller):**
```ruby
class DangerousController < ApplicationController
# ruleid:hardcoded-http-auth-in-controller
http_basic_authenticate_with :name => "dhh", :password => "secret", :except => :index
puts "do more stuff"
end
```
**Correct (Ruby - HTTP auth password from variable):**
```ruby
# ok:hardcoded-http-auth-in-controller
class OkController < ApplicationController
http_basic_authenticate_with :name => "dhh", :password => not_a_string, :except => :index
puts "do more stuff"
end
```
### Hardcoded Passwords
**Incorrect (Python Flask - hardcoded SECRET_KEY):**
```python
import os
import flask
app = flask.Flask(__name__)
# ruleid: avoid_hardcoded_config_SECRET_KEY
app.config.update(SECRET_KEY="aaaa")
# ruleid: avoid_hardcoded_config_SECRET_KEY
app.config["SECRET_KEY"] = '_5#y2L"F4Q8z\n\xec]/'
```
@@ -501,185 +107,63 @@ import os
import flask
app = flask.Flask(__name__)
# ok: avoid_hardcoded_config_SECRET_KEY
app.config.update(SECRET_KEY=os.getenv("SECRET_KEY"))
# ok: avoid_hardcoded_config_SECRET_KEY
app.config.update(SECRET_KEY=os.environ["SECRET_KEY"])
app.config["SECRET_KEY"] = os.environ["SECRET_KEY"]
```
**Incorrect (Python Django - empty password string):**
**Incorrect (Python - empty password string):**
```python
from models import UserProfile
def test_email_auth_backend_empty_password(user_profile: UserProfile) -> None:
user_profile = example_user('hamlet')
# ruleid: password-empty-string
def set_user_password(user_profile: UserProfile) -> None:
password = ""
user_profile.set_password(password)
user_profile.save()
# ruleid: password-empty-string
password = ''
user_profile.set_password(password)
user_profile.save()
```
**Correct (Python Django - non-empty password):**
**Correct (Python - password from secure source):**
```python
from models import UserProfile
def test_email_auth_backend_empty_password(user_profile: UserProfile) -> None:
user_profile = example_user('hamlet')
# ok: password-empty-string
password = "testpassword"
def set_user_password(user_profile: UserProfile, password: str) -> None:
user_profile.set_password(password)
user_profile.save()
```
**Incorrect (Python JWT - exposed credentials in token payload):**
### Third-Party Service Tokens
**Incorrect (JavaScript - hardcoded Stripe token):**
```javascript
const stripe = require('stripe');
const client = stripe('sk_test_20cbqx6v2hpftsbq203r36yqccazez');
```
**Correct (JavaScript - Stripe token from environment):**
```javascript
const stripe = require('stripe');
const client = stripe(process.env.STRIPE_SECRET_KEY);
```
**Incorrect (Python - hardcoded GitHub token):**
```python
import jwt
import requests
# ruleid: jwt-python-exposed-credentials
payload = {'foo': 'bar','password': 123}
def bad1(secret, value):
# ruleid: jwt-python-exposed-credentials
encoded = jwt.encode({'some': 'payload','password': value}, secret, algorithm='HS256')
return encoded
def bad3(secret, value):
# ruleid: jwt-python-exposed-credentials
pp = {'one': 'two','password': value}
encoded = jwt.encode(pp, secret, algorithm='HS256')
return encoded
headers = {"Authorization": "token ghp_emmtytndiqky5a98w0s98w36vfhiz6f7ed4c"}
response = requests.get("https://api.github.com/user", headers=headers)
```
**Correct (Python JWT - no credentials in token payload):**
**Correct (Python - GitHub token from environment):**
```python
import jwt
import os
import requests
def ok(secret_key):
# ok: jwt-python-exposed-credentials
encoded = jwt.encode({'some': 'payload'}, secret_key, algorithm='HS256')
return encoded
```
**Incorrect (Terraform - IAM credentials exposure):**
```hcl
resource "aws_iam_policy" "policy" {
name = "test_policy"
path = "/"
description = "My test policy"
policy = jsonencode({
Version = "2012-10-17"
Statement = [
{
# ruleid: no-iam-creds-exposure
Action = "sts:GetSessionToken"
Effect = "Allow"
Resource = "*"
},
]
})
}
resource "aws_iam_policy" "policy" {
name = "test_policy"
path = "/"
description = "My test policy"
policy = jsonencode({
Version = "2012-10-17"
Statement = [
{
# ruleid: no-iam-creds-exposure
Action = ["ec2:GetPasswordData"]
Effect = "Allow"
Resource = "*"
},
]
})
}
data aws_iam_policy_document "policy" {
statement {
# ruleid: no-iam-creds-exposure
actions = ["chime:CreateApiKey"]
principals {
type = "AWS"
identifiers = ["*"]
}
resources = ["*"]
}
}
```
**Correct (Terraform - IAM policy without credentials exposure):**
```hcl
resource "aws_iam_user_policy" "lb_ro" {
name = "test"
user = aws_iam_user.lb.name
policy = jsonencode({
Version = "2012-10-17"
Statement = [
{
# ok: no-iam-creds-exposure
Action = [
"ec2:Describe*",
]
Effect = "Allow"
Resource = "*"
},
]
})
}
data aws_iam_policy_document "policy" {
statement {
# ok: no-iam-creds-exposure
actions = ["ec2:Describe"]
resources = ["*"]
}
}
resource "aws_iam_policy" "policy" {
name = "test_policy"
path = "/"
description = "My test policy"
policy = jsonencode({
Version = "2012-10-17"
Statement = [
{
# ok: no-iam-creds-exposure - Deny effect
Action = ["ec2:GetPasswordData"]
Effect = "Deny"
Resource = "*"
},
]
})
}
data aws_iam_policy_document "policy" {
statement {
# ok: no-iam-creds-exposure - Deny effect
actions = ["chime:CreateApiKey"]
principals {
type = "AWS"
identifiers = ["*"]
}
resources = ["*"]
effect = "Deny"
}
}
headers = {"Authorization": f"token {os.environ['GITHUB_TOKEN']}"}
response = requests.get("https://api.github.com/user", headers=headers)
```
skills/code-security/rules/ssrf.md
+106 -1461
View File
File diff suppressed because it is too large Load Diff
skills/code-security/rules/terraform-aws.md
+99 -663
View File
@@ -5,758 +5,194 @@ impact: HIGH
## Secure AWS Terraform Configurations
This guide provides security best practices for AWS Terraform configurations. Following these patterns helps prevent common security misconfigurations that could expose your infrastructure to attacks.
Security best practices for AWS Terraform configurations to prevent common misconfigurations.
**Incorrect (EC2 - instance with public IP):**
### S3 Encryption
**Incorrect:**
```hcl
# ruleid: aws-ec2-has-public-ip
resource "aws_instance" "public" {
ami = "ami-12345"
instance_type = "t3.micro"
associate_public_ip_address = true
resource "aws_s3_bucket_object" "fail" {
bucket = aws_s3_bucket.bucket.bucket
key = "my-object"
content = "data"
}
```
**Correct (EC2 - instance without public IP):**
**Correct:**
```hcl
resource "aws_instance" "private" {
ami = "ami-12345"
instance_type = "t3.micro"
associate_public_ip_address = false
resource "aws_s3_bucket_object" "pass" {
bucket = aws_s3_bucket.bucket.bucket
key = "my-object"
content = "data"
kms_key_id = aws_kms_key.example.arn
}
```
**Incorrect (EC2 - launch template with public IP):**
### IAM Overly Permissive Policies
**Incorrect (wildcard admin):**
```hcl
# ruleid: aws-ec2-has-public-ip
resource "aws_launch_template" "public" {
image_id = "ami-12345"
instance_type = "t3.micro"
network_interfaces {
associate_public_ip_address = true
}
resource "aws_iam_policy" "fail" {
policy = <<POLICY
{"Version":"2012-10-17","Statement":[{"Action":"*","Effect":"Allow","Resource":"*"}]}
POLICY
}
```
**Correct (EC2 - launch template without public IP):**
**Correct (least privilege):**
```hcl
resource "aws_launch_template" "private" {
image_id = "ami-12345"
instance_type = "t3.micro"
network_interfaces {
associate_public_ip_address = false
}
resource "aws_iam_policy" "pass" {
policy = <<POLICY
{"Version":"2012-10-17","Statement":[{"Action":["s3:GetObject*"],"Effect":"Allow","Resource":"arn:aws:s3:::bucket/*"}]}
POLICY
}
```
**Incorrect (EC2 - security group allowing public SSH access):**
**Incorrect (wildcard AssumeRole):**
```hcl
# ruleid: aws-ec2-security-group-allows-public-ingress
resource "aws_security_group_rule" "fail_open_1" {
type = "ingress"
protocol = "tcp"
from_port = 22
to_port = 22
cidr_blocks = ["0.0.0.0/0"]
resource "aws_iam_role" "fail" {
assume_role_policy = <<POLICY
{"Version":"2012-10-17","Statement":[{"Effect":"Allow","Principal":{"AWS":"*"},"Action":"sts:AssumeRole"}]}
POLICY
}
```
**Correct (restricted AssumeRole):**
```hcl
resource "aws_security_group" "fail_open_1" {
vpc_id = aws_vpc.example.id
# ruleid: aws-ec2-security-group-allows-public-ingress
ingress {
protocol = "tcp"
from_port = 22
to_port = 22
cidr_blocks = ["0.0.0.0/0"]
}
resource "aws_iam_role" "pass" {
assume_role_policy = <<POLICY
{"Version":"2012-10-17","Statement":[{"Effect":"Allow","Principal":{"AWS":"arn:aws:iam::123456789012:root"},"Action":"sts:AssumeRole"}]}
POLICY
}
```
**Correct (EC2 - security group with restricted CIDR):**
### Unencrypted Storage
**Incorrect (EBS):**
```hcl
# ok: aws-ec2-security-group-allows-public-ingress
resource "aws_security_group_rule" "pass_inside_private_network_1" {
type = "ingress"
protocol = "tcp"
from_port = 22
to_port = 22
cidr_blocks = ["10.0.0.0/8"]
}
```
```hcl
resource "aws_security_group" "pass_inside_private_network_1" {
vpc_id = aws_vpc.example.id
# ok: aws-ec2-security-group-allows-public-ingress
ingress {
protocol = "tcp"
from_port = 22
to_port = 22
cidr_blocks = ["10.0.0.0/8"]
}
}
```
**Incorrect (EBS - unencrypted volume):**
```hcl
# ruleid: aws-ebs-volume-unencrypted
resource "aws_ebs_volume" "fail_1" {
availability_zone = "us-west-2a"
}
# ruleid: aws-ebs-volume-unencrypted
resource "aws_ebs_volume" "fail_2" {
resource "aws_ebs_volume" "fail" {
availability_zone = "us-west-2a"
encrypted = false
}
```
**Correct (EBS - encrypted volume):**
**Correct (EBS):**
```hcl
# ok: aws-ebs-volume-unencrypted
resource "aws_ebs_volume" "pass" {
availability_zone = "us-west-2a"
encrypted = true
}
```
**Incorrect (S3 - object without CMK encryption):**
**Incorrect (RDS no backup):**
```hcl
# ruleid: aws-s3-bucket-object-encrypted-with-cmk
resource "aws_s3_bucket_object" "fail" {
bucket = aws_s3_bucket.object_bucket.bucket
key = "tf-testing-obj-%[1]d-encrypted"
content = "Keep Calm and Carry On"
content_type = "text/plain"
resource "aws_db_instance" "fail" { backup_retention_period = 0 }
```
**Correct (RDS with backup):**
```hcl
resource "aws_db_instance" "pass" { backup_retention_period = 35 }
```
**Incorrect (DynamoDB):**
```hcl
resource "aws_dynamodb_table" "fail" {
name = "Table"; hash_key = "Id"
attribute { name = "Id"; type = "S" }
}
```
**Correct (S3 - object with CMK encryption):**
**Correct (DynamoDB with CMK):**
```hcl
resource "aws_s3_bucket_object" "pass" {
bucket = aws_s3_bucket.object_bucket.bucket
key = "tf-testing-obj-%[1]d-encrypted"
content = "Keep Calm and Carry On"
content_type = "text/plain"
kms_key_id = aws_kms_key.example.arn
resource "aws_dynamodb_table" "pass" {
name = "Table"; hash_key = "Id"
attribute { name = "Id"; type = "S" }
server_side_encryption { enabled = true; kms_key_arn = "arn:aws:kms:..." }
}
```
**Incorrect (RDS - without backup retention):**
**Incorrect (SQS/SNS):**
```hcl
# ruleid: aws-rds-backup-no-retention
resource "aws_rds_cluster" "fail2" {
backup_retention_period = 0
}
# ruleid: aws-rds-backup-no-retention
resource "aws_db_instance" "fail" {
backup_retention_period = 0
}
```
**Correct (RDS - with backup retention):**
```hcl
resource "aws_rds_cluster" "pass" {
backup_retention_period = 35
}
resource "aws_db_instance" "pass" {
backup_retention_period = 35
}
```
**Incorrect (IAM - policy with wildcard admin access):**
```hcl
resource "aws_iam_policy" "fail3" {
name = "fail3"
path = "/"
# ruleid: aws-iam-admin-policy
policy = <<POLICY
{
"Statement": [
{
"Action": "*",
"Effect": "Allow",
"Resource": "*",
"Sid": ""
}
],
"Version": "2012-10-17"
}
POLICY
}
```
**Correct (IAM - policy with specific permissions):**
```hcl
resource "aws_iam_policy" "pass1" {
name = "pass1"
path = "/"
policy = <<POLICY
{
"Statement": [
{
"Action": [
"s3:ListBucket*",
"s3:HeadBucket",
"s3:Get*"
],
"Effect": "Allow",
"Resource": [
"arn:aws:s3:::b1",
"arn:aws:s3:::b1/*",
"arn:aws:s3:::b2",
"arn:aws:s3:::b2/*"
],
"Sid": ""
},
{
"Action": "s3:PutObject*",
"Effect": "Allow",
"Resource": "arn:aws:s3:::b1/*",
"Sid": ""
}
],
"Version": "2012-10-17"
}
POLICY
}
```
**Incorrect (IAM - wildcard AssumeRole policy):**
```hcl
resource "aws_iam_role" "bad" {
name = var.role_name
# ruleid: wildcard-assume-role
assume_role_policy = <<POLICY
{
"Version": "2012-10-17",
"Statement": [
{
"Effect": "Allow",
"Principal": {
"AWS": "*"
},
"Action": "sts:AssumeRole",
"Condition": {}
}
]
}
POLICY
}
```
**Correct (IAM - restricted AssumeRole policy):**
```hcl
resource "aws_iam_role" "ok" {
name = var.role_name
# ok: wildcard-assume-role
assume_role_policy = <<POLICY
{
"Version": "2012-10-17",
"Statement": [
{
"Effect": "Allow",
"Principal": {
"AWS": "*"
},
"Action": "arn:aws:iam::${data.aws_caller_identity.current.account_id}:root",
"Condition": {}
}
]
}
POLICY
}
```
**Incorrect (Lambda - with hard-coded credentials):**
```hcl
resource "aws_lambda_function" "fail" {
function_name = "stest-env"
role = ""
runtime = "python3.8"
environment {
variables = {
# ruleid: aws-lambda-environment-credentials
AWS_ACCESS_KEY_ID = "AKIAIOSFODNN7EXAMPLE",
# ruleid: aws-lambda-environment-credentials
AWS_SECRET_ACCESS_KEY = "wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY",
AWS_DEFAULT_REGION = "us-west-2"
}
}
}
```
**Correct (Lambda - without credentials):**
```hcl
resource "aws_lambda_function" "pass" {
function_name = "test-env"
role = ""
runtime = "python3.8"
environment {
variables = {
AWS_DEFAULT_REGION = "us-west-2"
}
}
}
```
**Incorrect (Lambda - permission without source ARN):**
```hcl
# ruleid: aws-lambda-permission-unrestricted-source-arn
resource "aws_lambda_permission" "fail_1" {
statement_id = "AllowExecutionFromSNS"
action = "lambda:InvokeFunction"
function_name = aws_lambda_function.func.function_name
principal = "sns.amazonaws.com"
}
# ruleid: aws-lambda-permission-unrestricted-source-arn
resource "aws_lambda_permission" "fail_3" {
statement_id = "AllowMyDemoAPIInvoke"
action = "lambda:InvokeFunction"
function_name = "MyDemoFunction"
principal = "apigateway.amazonaws.com"
}
```
**Correct (Lambda - permission with source ARN):**
```hcl
# ok: aws-lambda-permission-unrestricted-source-arn
resource "aws_lambda_permission" "pass_1" {
statement_id = "AllowExecutionFromSNS"
action = "lambda:InvokeFunction"
function_name = aws_lambda_function.func.function_name
principal = "sns.amazonaws.com"
source_arn = aws_sns_topic.default.arn
}
# ok: aws-lambda-permission-unrestricted-source-arn
resource "aws_lambda_permission" "pass_3" {
statement_id = "AllowMyDemoAPIInvoke"
action = "lambda:InvokeFunction"
function_name = "MyDemoFunction"
principal = "apigateway.amazonaws.com"
# The /* part allows invocation from any stage, method and resource path
# within API Gateway.
source_arn = "${aws_api_gateway_rest_api.MyDemoAPI.execution_arn}/*"
}
```
**Incorrect (KMS - key without rotation):**
```hcl
# ruleid: aws-kms-no-rotation
resource "aws_kms_key" "fail1" {
description = "KMS key 1"
deletion_window_in_days = 10
}
# ruleid: aws-kms-no-rotation
resource "aws_kms_key" "fail2" {
description = "KMS key 1"
deletion_window_in_days = 10
enable_key_rotation = false
}
```
**Correct (KMS - key with rotation enabled):**
```hcl
resource "aws_kms_key" "pass1" {
description = "KMS key 1"
deletion_window_in_days = 10
enable_key_rotation = true
}
```
**Incorrect (SQS - unencrypted queue):**
```hcl
# ruleid: aws-sqs-queue-unencrypted
resource "aws_sqs_queue" "fail_1" {
name = "terraform-example-queue"
}
# ruleid: aws-sqs-queue-unencrypted
resource "aws_sqs_queue" "fail_2" {
name = "terraform-example-queue"
sqs_managed_sse_enabled = false
}
```
**Correct (SQS - encrypted queue):**
```hcl
# ok: aws-sqs-queue-unencrypted
resource "aws_sqs_queue" "pass_1" {
name = "terraform-example-queue"
sqs_managed_sse_enabled = true
}
# ok: aws-sqs-queue-unencrypted
resource "aws_sqs_queue" "pass_2" {
name = "terraform-example-queue"
kms_master_key_id = "alias/aws/sqs"
kms_data_key_reuse_period_seconds = 300
}
```
**Incorrect (SNS - unencrypted topic):**
```hcl
# ruleid: aws-sns-topic-unencrypted
resource "aws_sqs_queue" "fail" { name = "queue" }
resource "aws_sns_topic" "fail" {}
```
**Correct (SNS - encrypted topic):**
**Correct (SQS/SNS encrypted):**
```hcl
# ok: aws-sns-topic-unencrypted
resource "aws_sns_topic" "pass" {
kms_master_key_id = "someKey"
resource "aws_sqs_queue" "pass" { name = "queue"; sqs_managed_sse_enabled = true }
resource "aws_sns_topic" "pass" { kms_master_key_id = "alias/aws/sns" }
```
### Network Security
**Incorrect (public SSH):**
```hcl
resource "aws_security_group_rule" "fail" {
type = "ingress"; protocol = "tcp"; from_port = 22; to_port = 22
cidr_blocks = ["0.0.0.0/0"]
}
```
**Incorrect (DynamoDB - without CMK encryption):**
**Correct (restricted CIDR):**
```hcl
# ruleid: aws-dynamodb-table-unencrypted
resource "aws_dynamodb_table" "default" {
name = "GameScores"
billing_mode = "PROVISIONED"
read_capacity = 20
write_capacity = 20
hash_key = "UserId"
range_key = "UserId"
attribute {
name = "UserId"
type = "S"
}
}
# ruleid: aws-dynamodb-table-unencrypted
resource "aws_dynamodb_table" "encrypted_no_cmk" {
name = "GameScores"
billing_mode = "PROVISIONED"
read_capacity = 20
write_capacity = 20
hash_key = "UserId"
range_key = "UserId"
attribute {
name = "UserId"
type = "S"
}
server_side_encryption {
enabled = true
}
resource "aws_security_group_rule" "pass" {
type = "ingress"; protocol = "tcp"; from_port = 22; to_port = 22
cidr_blocks = ["10.0.0.0/8"]
}
```
**Correct (DynamoDB - with CMK encryption):**
**Incorrect (public IP):**
```hcl
resource "aws_dynamodb_table" "cmk" {
name = "GameScores"
billing_mode = "PROVISIONED"
read_capacity = 20
write_capacity = 20
hash_key = "UserId"
range_key = "UserId"
attribute {
name = "UserId"
type = "S"
}
server_side_encryption {
enabled = true
kms_key_arn = "arn:aws:kms:us-west-2:123456789012:key/1234abcd-12ab-34cd-56ef-1234567890ab"
}
resource "aws_instance" "fail" {
ami = "ami-12345"; instance_type = "t3.micro"
associate_public_ip_address = true
}
```
**Incorrect (ECR - with mutable tags):**
**Correct (no public IP):**
```hcl
# ruleid: aws-ecr-mutable-image-tags
resource "aws_ecr_repository" "fail_1" {
name = "example"
}
# ruleid: aws-ecr-mutable-image-tags
resource "aws_ecr_repository" "fail_2" {
name = "example"
image_tag_mutability = "MUTABLE"
resource "aws_instance" "pass" {
ami = "ami-12345"; instance_type = "t3.micro"
associate_public_ip_address = false
}
```
**Correct (ECR - with immutable tags):**
### Key Management
**Incorrect (KMS no rotation):**
```hcl
# ok: aws-ecr-mutable-image-tags
resource "aws_ecr_repository" "pass" {
name = "example"
image_tag_mutability = "IMMUTABLE"
}
resource "aws_kms_key" "fail" { enable_key_rotation = false }
```
**Incorrect (CloudTrail - without encryption):**
**Correct (KMS with rotation):**
```hcl
# ruleid: aws-cloudtrail-encrypted-with-cmk
resource "aws_cloudtrail" "fail" {
name = "TRAIL"
s3_bucket_name = aws_s3_bucket.test.id
include_global_service_events = true
}
resource "aws_kms_key" "pass" { enable_key_rotation = true }
```
**Correct (CloudTrail - with CMK encryption):**
**Incorrect (CloudTrail):**
```hcl
resource "aws_cloudtrail" "fail" { name = "trail"; s3_bucket_name = "bucket" }
```
**Correct (CloudTrail encrypted):**
```hcl
resource "aws_cloudtrail" "pass" {
name = "TRAIL"
s3_bucket_name = aws_s3_bucket.test.id
include_global_service_events = true
kms_key_id = aws_kms_key.test.arn
name = "trail"; s3_bucket_name = "bucket"; kms_key_id = aws_kms_key.key.arn
}
```
**Incorrect (Elasticsearch - with insecure TLS):**
```hcl
# ruleid: aws-elasticsearch-insecure-tls-version
resource "aws_elasticsearch_domain" "badCode" {
domain_name = "badCode"
domain_endpoint_options {
enforce_https = true
tls_security_policy = "Policy-Min-TLS-1-0-2019-07"
}
}
```
**Correct (Elasticsearch - with TLS 1.2):**
```hcl
resource "aws_elasticsearch_domain" "okCode" {
domain_name = "okCode"
domain_endpoint_options {
enforce_https = true
tls_security_policy = "Policy-Min-TLS-1-2-2019-07"
}
}
```
**Incorrect (Load Balancer - with insecure TLS):**
```hcl
resource "aws_lb_listener" "https_2016" {
load_balancer_arn = var.aws_lb_arn
protocol = "HTTPS"
port = "443"
# ruleid: insecure-load-balancer-tls-version
ssl_policy = "ELBSecurityPolicy-2016-08"
certificate_arn = var.certificate_arn
default_action {
type = "forward"
target_group_arn = var.aws_lb_target_group_arn
}
}
resource "aws_lb_listener" "http" {
load_balancer_arn = var.aws_lb_arn
# ruleid: insecure-load-balancer-tls-version
protocol = "HTTP"
port = "80"
default_action {
type = "forward"
target_group_arn = var.aws_lb_target_group_arn
}
}
```
**Correct (Load Balancer - with TLS 1.2+):**
```hcl
resource "aws_lb_listener" "https_fs_1_2" {
load_balancer_arn = var.aws_lb_arn
protocol = "HTTPS"
port = "443"
# ok: insecure-load-balancer-tls-version
ssl_policy = "ELBSecurityPolicy-FS-1-2-Res-2019-08"
certificate_arn = var.certificate_arn
default_action {
type = "forward"
target_group_arn = var.aws_lb_target_group_arn
}
}
resource "aws_lb_listener" "http_redirect" {
load_balancer_arn = var.aws_lb_arn
# ok: insecure-load-balancer-tls-version
protocol = "HTTP"
port = "80"
default_action {
type = "redirect"
redirect {
port = "443"
protocol = "HTTPS"
status_code = "HTTP_301"
}
}
}
```
**Incorrect (VPC - subnet with public IP assignment):**
```hcl
# ruleid: aws-subnet-has-public-ip-address
resource "aws_subnet" "fail_1" {
vpc_id = "vpc-123456"
map_public_ip_on_launch = true
}
# ruleid: aws-subnet-has-public-ip-address
resource "aws_default_subnet" "fail_2" {
availability_zone = "us-west-2a"
}
```
**Correct (VPC - subnet without public IP assignment):**
```hcl
# ok: aws-subnet-has-public-ip-address
resource "aws_subnet" "pass_1" {
vpc_id = "vpc-123456"
}
# ok: aws-subnet-has-public-ip-address
resource "aws_subnet" "pass_2" {
vpc_id = "vpc-123456"
map_public_ip_on_launch = false
}
# ok: aws-subnet-has-public-ip-address
resource "aws_default_subnet" "pass_3" {
availability_zone = "us-west-2a"
map_public_ip_on_launch = false
}
```
**Incorrect (CodeBuild - with unencrypted artifacts):**
```hcl
resource "aws_codebuild_project" "fail_1" {
name = "test-project"
service_role = aws_iam_role.example.arn
# ruleid: aws-codebuild-artifacts-unencrypted
artifacts {
encryption_disabled = true
type = "CODEPIPELINE"
}
environment {
compute_type = "BUILD_GENERAL1_SMALL"
image = "aws/codebuild/standard:1.0"
type = "LINUX_CONTAINER"
}
source {
type = "GITHUB"
location = "https://github.com/mitchellh/packer.git"
git_clone_depth = 1
}
}
```
**Correct (CodeBuild - with encrypted artifacts):**
```hcl
resource "aws_codebuild_project" "pass_4" {
name = "test-project"
service_role = aws_iam_role.example.arn
# ok: aws-codebuild-artifacts-unencrypted
artifacts {
type = "CODEPIPELINE"
encryption_disabled = false
}
environment {
compute_type = "BUILD_GENERAL1_SMALL"
image = "aws/codebuild/standard:1.0"
type = "LINUX_CONTAINER"
}
source {
type = "GITHUB"
location = "https://github.com/mitchellh/packer.git"
git_clone_depth = 1
}
}
```
**Incorrect (AWS Provider - with hard-coded credentials):**
### Credentials
**Incorrect (hardcoded):**
```hcl
provider "aws" {
region = "us-west-2"
access_key = "AKIAEXAMPLEKEY"
# ruleid: aws-provider-static-credentials
secret_key = "randomcharactersabcdef"
profile = "customprofile"
region = "us-west-2"; access_key = "AKIAEXAMPLE"; secret_key = "secret"
}
```
**Correct (AWS Provider - using shared credentials file):**
**Correct (external credentials):**
```hcl
# ok: aws-provider-static-credentials
provider "aws" {
region = "us-west-2"
shared_credentials_file = "/Users/tf_user/.aws/creds"
profile = "customprofile"
region = "us-west-2"; shared_credentials_file = "~/.aws/creds"; profile = "myprofile"
}
```
skills/code-security/rules/terraform-azure.md
+209 -1633
View File
File diff suppressed because it is too large Load Diff
skills/code-security/rules/terraform-gcp.md
+134 -1913
View File
File diff suppressed because it is too large Load Diff
skills/code-security/rules/xss.md
+76 -873
View File
File diff suppressed because it is too large Load Diff
skills/code-security/rules/xxe.md
+54 -1297
View File
File diff suppressed because it is too large Load Diff