mirror of
https://github.com/trailofbits/skills.git
synced 2026-09-14 14:28:48 +08:00
Thomas Chauchefoin
82e8e0ad77
* insecure-defaults: convert the skill to a dynamic workflow Rewrites the plugin as a four-phase workflow: recon profiles the target, parallel sweeps collect candidates, a refuting pass adjudicates them, and a report assigns severity with coverage accounting. The skill is removed and `/insecure-defaults:audit [path]` is the only entry point, since the workflow needs the command to locate its detection corpora. It also adds offline tests: scenarios over the workflow's control flow, a mutation self-test that proves they bite, and a check that every documented example is matched by a seed pattern. This does not replace running the command end-to-end against a real codebase. Bumps to 2.0.0. * insecure-defaults: run the node suites in CI The harness and seed-coverage checks ran only when someone remembered to. CI's shell-suite discovery matches plugins/*/tests/run_*.sh, so wrap the three node invocations in run_seeds.sh and both the lint workflow and `make shell-suites` pick them up with no changes to either. Without this, adding a row to CATEGORIES without its references/<id>.json passes CI and then aborts every real run with corpus-unreadable. No setup-node step: ubuntu-latest ships Node, and the scripts are plain CommonJS with no dependencies. The command -v guard makes a missing interpreter a loud failure rather than a suite that quietly does not run. * insecure-defaults: abort when the verify phase adjudicates nothing If every verify batch died, confirmed was empty and the run returned no-findings-confirmed, which commands/audit.md considers a completed audit. Guard on unadjudicated.length === candidates.length and return verify-failed, carrying coverage so the caller sees what went unjudged. Adds a scenario for both failure shapes (all agents dead, all verdict lists empty) and two mutations covering the guard firing and over-firing. * insecure-defaults: report per-category scan counts The zero-scanned guard is on the sum, so five failed searches beside one that worked cleared it and categories_run listed all six. Add files_scanned_by_category and unsearched_categories to coverage, keyed off CATEGORIES so a dead sweep counts as 0, and have the report name them under a Not searched heading. Adds a scenario covering a searching sweep, a zero-file one and a dead one, plus three mutations. * insecure-defaults: count sweep failures against the category list The corpus-unreadable note and the seed-only log divided by the sweeps that returned, so with sweeps dead the ratio read 1/1 rather than 1/6. * insecure-defaults: drop the assertion-count floor from the harness * insecure-defaults: guard a dead report agent agent() returns null on terminal failure, so a report agent that died returned status "findings" with no report and the caller printed nothing while the findings sat in the structured return. Return report-failed with the findings and coverage, and have the command render them. * insecure-defaults: stop labelling a genuine clean run a failure Step 3 accepted findings/no-findings-confirmed and called every other status an incomplete audit, so no-candidates, the deliberate honest-negative status, told the user the run failed. It is now a per-status table. * insecure-defaults: anchor the noisiest seeds (DES|RC4|...) matched NODES and MODES, 0o?(666|777|...) matched any digits, and getMessage() matched all Java exception handling. On the Python stdlib the first two drop from 147 and 228 matching lines to 0 and 84. random. and getMessage() can't be fixed by anchoring, so they now require context: a security-material identifier near the RNG call, and concatenation into a string literal for getMessage(). seed-coverage.js confirms all 18 documented VULNERABLE examples still match. * insecure-defaults: indent the seed wrapper the way shfmt wants --------- Co-authored-by: kz-tob <kara.zaffarano@trailofbits.com>
2.1 KiB
2.1 KiB
Weak Cryptographic Defaults
Report when: A broken or non-cryptographic primitive standing in for a security-relevant one: password hashing, token generation, encryption, signature verification.
Skip when: Checksums, ETags, cache keys, deduplication hashes, test vectors. Non-security shuffling or sampling.
The algorithm alone is never the finding. hashlib.md5 over a cache key is fine; the same call over a password is not. Trace to the use site before filing.
VULNERABLE - Report These
MD5 for password hashing
# File: src/auth/passwords.py
import hashlib
def hash_password(password):
"""Hash user password"""
return hashlib.md5(password.encode()).hexdigest()
Why vulnerable: MD5 is cryptographically broken. Rainbow tables exist. Use bcrypt/Argon2.
DES encryption for sensitive data
// File: Encryption.java
public static byte[] encrypt(String data, byte[] key) {
Cipher cipher = Cipher.getInstance("DES/ECB/PKCS5Padding");
SecretKeySpec secretKey = new SecretKeySpec(key, "DES");
cipher.init(Cipher.ENCRYPT_MODE, secretKey);
return cipher.doFinal(data.getBytes());
}
Why vulnerable: DES has 56-bit keys (brute-forceable). ECB mode leaks patterns.
SHA1 for signature verification
// File: webhooks.js
function verifySignature(payload, signature) {
const hmac = crypto.createHmac('sha1', WEBHOOK_SECRET);
const computed = hmac.update(payload).digest('hex');
return computed === signature;
}
Why vulnerable: SHA1 collisions exist. Use SHA256 or better.
SECURE - Skip These
Weak crypto for non-security checksums
# File: src/utils/cache.py
import hashlib
def cache_key(data):
"""Generate cache key - not security-sensitive"""
return hashlib.md5(data.encode()).hexdigest() # OK - just for cache lookup
Modern crypto for passwords
# File: src/auth/passwords.py
import bcrypt
def hash_password(password):
return bcrypt.hashpw(password.encode(), bcrypt.gensalt())
Strong encryption
// File: Encryption.java
Cipher cipher = Cipher.getInstance("AES/GCM/NoPadding");
// 256-bit key, authenticated encryption