|
| 1 | +# Security |
| 2 | + |
| 3 | +This document describes security measures implemented in Tab Modifier. |
| 4 | + |
| 5 | +## CI/CD Security Scanning |
| 6 | + |
| 7 | +Tab Modifier uses multiple security scanning tools in the CI/CD pipeline to ensure code quality and security: |
| 8 | + |
| 9 | +### 1. ClamAV Malware Scan |
| 10 | +- **Purpose**: Detects viruses, trojans, and other malware in the codebase |
| 11 | +- **Frequency**: On every push to any branch |
| 12 | +- **Configuration**: `.github/workflows/ci.yml` - `clamav_malware_scan` job |
| 13 | +- **Coverage**: Scans all files except `node_modules`, `.git`, and `dist` |
| 14 | +- **Action on detection**: Pipeline fails if malware is detected |
| 15 | + |
| 16 | +### 2. Semgrep SAST (Static Application Security Testing) |
| 17 | +- **Purpose**: Detects security vulnerabilities and code quality issues |
| 18 | +- **Configuration**: `.github/workflows/ci.yml` - `semgrep_scan` job |
| 19 | +- **Results**: Uploaded to GitHub Security Dashboard as SARIF |
| 20 | + |
| 21 | +### 3. CodeQL SAST |
| 22 | +- **Purpose**: Advanced semantic code analysis for security vulnerabilities |
| 23 | +- **Configuration**: `.github/workflows/ci.yml` - `codeql_sast` job |
| 24 | +- **Queries**: `security-extended` and `security-and-quality` |
| 25 | +- **Results**: Uploaded to GitHub Security Dashboard |
| 26 | + |
| 27 | +### 4. Gitleaks Secret Scan |
| 28 | +- **Purpose**: Detects hardcoded secrets, API keys, and credentials |
| 29 | +- **Configuration**: `.github/workflows/ci.yml` - `gitleaks_scan` job |
| 30 | +- **Action**: Official `gitleaks/gitleaks-action@v2` |
| 31 | +- **Scope**: Full git history scan (`fetch-depth: 0`) |
| 32 | +- **Results**: Uploaded to GitHub Security Dashboard as SARIF |
| 33 | +- **Action on detection**: Pipeline fails if secrets are found |
| 34 | + |
| 35 | +### 5. Dependency Vulnerability Audit |
| 36 | +- **Purpose**: Checks for known vulnerabilities in npm/yarn dependencies |
| 37 | +- **Configuration**: `.github/workflows/ci.yml` - `dependency_audit` job |
| 38 | +- **Severity**: Fails on HIGH severity and above |
| 39 | +- **Tool**: `audit-ci` with yarn |
| 40 | + |
| 41 | +## ReDoS (Regular Expression Denial of Service) Protection |
| 42 | + |
| 43 | +### Background |
| 44 | + |
| 45 | +Tab Modifier allows users to create rules with regular expressions to match URLs. User-controlled regex patterns can potentially be exploited to cause ReDoS attacks, where malicious regex patterns with catastrophic backtracking can freeze the browser tab. |
| 46 | + |
| 47 | +### Implementation |
| 48 | + |
| 49 | +We've implemented multi-layered protection against ReDoS attacks: |
| 50 | + |
| 51 | +#### 1. Pattern Validation (`src/common/regex-safety.ts`) |
| 52 | + |
| 53 | +The `_isRegexPatternSafe()` function validates regex patterns before execution: |
| 54 | + |
| 55 | +- **Nested Quantifiers**: Blocks patterns like `(a+)+`, `(a*)*`, `(a{1,5})+` that cause exponential backtracking |
| 56 | +- **Consecutive Quantifiers**: Blocks patterns like `a**`, `a+*` that are invalid or dangerous |
| 57 | +- **Overlapping Alternatives**: Blocks patterns like `(a|a)*`, `(x+|x+y+)*` that create unnecessary backtracking |
| 58 | +- **Length Limits**: Rejects patterns longer than 1000 characters |
| 59 | +- **Syntax Validation**: Ensures the pattern is a valid regex |
| 60 | + |
| 61 | +#### 2. Safe Execution |
| 62 | + |
| 63 | +The `_safeRegexTestSync()` function: |
| 64 | +1. Validates the pattern before execution |
| 65 | +2. Catches and logs any execution errors |
| 66 | +3. Returns `false` for unsafe patterns instead of executing them |
| 67 | + |
| 68 | +### Usage |
| 69 | + |
| 70 | +Instead of using `new RegExp(pattern).test(url)` directly, always use: |
| 71 | + |
| 72 | +```typescript |
| 73 | +import { _safeRegexTestSync } from './regex-safety.ts'; |
| 74 | + |
| 75 | +// Safe regex execution |
| 76 | +const matches = _safeRegexTestSync(userPattern, url); |
| 77 | +``` |
| 78 | + |
| 79 | +### Examples of Blocked Patterns |
| 80 | + |
| 81 | +**Dangerous patterns that are blocked:** |
| 82 | +```regex |
| 83 | +(a+)+ # Nested quantifiers |
| 84 | +(a*)* # Nested quantifiers |
| 85 | +(a|a)* # Overlapping alternatives |
| 86 | +(x+|x+y+)* # Overlapping alternatives with quantifiers |
| 87 | +a++ # Consecutive quantifiers |
| 88 | +``` |
| 89 | + |
| 90 | +**Safe patterns that are allowed:** |
| 91 | +```regex |
| 92 | +example\.com # Simple literal match |
| 93 | +^https://[a-z]+\.example\.com # Character classes with quantifiers |
| 94 | +[0-9]{1,4} # Bounded quantifiers |
| 95 | +(?!MOUNCE).*version= # Negative lookahead |
| 96 | +``` |
| 97 | + |
| 98 | +### Testing |
| 99 | + |
| 100 | +Comprehensive tests are in `src/common/regex-safety.test.js` covering: |
| 101 | +- Safe pattern validation |
| 102 | +- Dangerous pattern detection and blocking |
| 103 | +- Real-world regex patterns from existing rules |
| 104 | +- Edge cases and error handling |
| 105 | + |
| 106 | +### Best Practices |
| 107 | + |
| 108 | +1. **Prefer simpler detection methods** when possible: |
| 109 | + - Use `CONTAINS` for substring matching |
| 110 | + - Use `STARTS_WITH` / `ENDS_WITH` for prefix/suffix matching |
| 111 | + - Use `EXACT` for exact matching |
| 112 | + - Use `REGEX` only when pattern matching is truly needed |
| 113 | + |
| 114 | +2. **Write safe regex patterns**: |
| 115 | + - Avoid nested quantifiers |
| 116 | + - Use bounded quantifiers (`{1,5}`) instead of unbounded (`*`, `+`) |
| 117 | + - Keep patterns as simple as possible |
| 118 | + - Test patterns with the safety validator |
| 119 | + |
| 120 | +3. **Document complex patterns**: |
| 121 | + - Add comments explaining what the pattern matches |
| 122 | + - Include test cases for complex patterns |
| 123 | + |
| 124 | +## Missing Regex Anchors in URL/Title Matchers |
| 125 | + |
| 126 | +### Background |
| 127 | + |
| 128 | +The `url_matcher` and `title_matcher` features allow users to extract parts of URLs and page titles using regular expressions with capture groups. When regex patterns don't use anchors (`^` for start, `$` for end), they can match anywhere in the input string, potentially causing unexpected behavior. |
| 129 | + |
| 130 | +### Context: Not a Critical Security Issue |
| 131 | + |
| 132 | +In Tab Modifier, `url_matcher` and `title_matcher` are used to **extract content** for display in tab titles, not for security-critical validation like URL redirection or authentication. Therefore, missing anchors here represent a **usability concern** rather than a security vulnerability. |
| 133 | + |
| 134 | +However, to prevent unexpected matches and follow security best practices, we recommend using anchored patterns. |
| 135 | + |
| 136 | +### Recommendations |
| 137 | + |
| 138 | +#### Use Anchors When Possible |
| 139 | + |
| 140 | +**Unanchored pattern (may match unexpectedly):** |
| 141 | +```regex |
| 142 | +https:\/\/example\.com\/(.+) |
| 143 | +# Could match: "evil.com?redirect=https://example.com/test" |
| 144 | +``` |
| 145 | + |
| 146 | +**Anchored pattern (matches precisely):** |
| 147 | +```regex |
| 148 | +^https:\/\/example\.com\/(.+) |
| 149 | +# Only matches URLs starting with https://example.com/ |
| 150 | +``` |
| 151 | + |
| 152 | +#### Examples of Safe Patterns |
| 153 | + |
| 154 | +**URL Matcher for GitHub repositories:** |
| 155 | +```regex |
| 156 | +^https:\/\/github\.com\/([A-Za-z0-9_-]+)\/([A-Za-z0-9_-]+) |
| 157 | +``` |
| 158 | + |
| 159 | +**URL Matcher for query parameters:** |
| 160 | +```regex |
| 161 | +[?&]date=([0-9]{4}-[0-9]{2}-[0-9]{2}) |
| 162 | +# Note: This doesn't need ^ anchor as we're matching a specific parameter |
| 163 | +``` |
| 164 | + |
| 165 | +**Title Matcher with full anchors:** |
| 166 | +```regex |
| 167 | +^[a-z]*@gmail\.com$ |
| 168 | +``` |
| 169 | + |
| 170 | +#### When Anchors Are Optional |
| 171 | + |
| 172 | +For patterns that specifically target **substrings** (like query parameters or path segments), anchors may not be needed: |
| 173 | + |
| 174 | +```regex |
| 175 | +# Extracting query parameter - no anchor needed |
| 176 | +[?&]id=([0-9]+) |
| 177 | +
|
| 178 | +# Extracting date from URL - no anchor needed |
| 179 | +date=([0-9]{4}-[0-9]{2}-[0-9]{2}) |
| 180 | +``` |
| 181 | + |
| 182 | +### Implementation |
| 183 | + |
| 184 | +The regex safety checks in `src/content.js` already validate patterns before execution. To enhance this: |
| 185 | + |
| 186 | +1. **Pattern Validation**: The `isRegexSafe()` function checks for dangerous patterns |
| 187 | +2. **Safe Execution**: The `createSafeRegex()` function creates validated regex instances |
| 188 | +3. **Error Handling**: All regex operations are wrapped in try-catch blocks |
| 189 | + |
| 190 | +### Best Practices for URL/Title Matchers |
| 191 | + |
| 192 | +1. **Use anchors (`^`, `$`) when matching complete URLs or titles** |
| 193 | + ```regex |
| 194 | + ✅ ^https:\/\/example\.com\/path$ |
| 195 | + ❌ https:\/\/example\.com\/path |
| 196 | + ``` |
| 197 | + |
| 198 | +2. **Be specific with your patterns** |
| 199 | + ```regex |
| 200 | + ✅ ^https:\/\/github\.com\/[A-Za-z0-9_-]+\/[A-Za-z0-9_-]+ |
| 201 | + ❌ .+github.com.+ |
| 202 | + ``` |
| 203 | + |
| 204 | +3. **Use character classes instead of wildcards when possible** |
| 205 | + ```regex |
| 206 | + ✅ [A-Za-z0-9_-]+ |
| 207 | + ❌ .+ |
| 208 | + ``` |
| 209 | + |
| 210 | +4. **Test your patterns with the regex safety validator** |
| 211 | + - Patterns are automatically validated before use |
| 212 | + - Check browser console for validation warnings |
| 213 | + |
| 214 | +5. **Consider the use case** |
| 215 | + - For extracting specific parts: anchors may be optional |
| 216 | + - For matching the entire URL/title: always use anchors |
| 217 | + |
| 218 | +### References |
| 219 | + |
| 220 | +- [OWASP: Regular Expression Denial of Service](https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS) |
| 221 | +- [Semgrep Rule: detect-non-literal-regexp](https://semgrep.dev/r/javascript.lang.security.audit.detect-non-literal-regexp) |
| 222 | +- [CodeQL: Missing Regular Expression Anchor](https://codeql.github.com/codeql-query-help/javascript/js-regex-missing-anchor/) |
| 223 | +- [OWASP: Server Side Request Forgery](https://owasp.org/www-community/attacks/Server_Side_Request_Forgery) |
0 commit comments