|
| 1 | +# Critical Issues Report - WikiContest |
| 2 | + |
| 3 | +**Generated:** $(date) |
| 4 | +**Status:** ⚠️ CRITICAL - Immediate Action Required |
| 5 | + |
| 6 | +## Executive Summary |
| 7 | + |
| 8 | +This report identifies **6 critical security and configuration issues** that must be addressed before production deployment. These issues pose significant security risks including: |
| 9 | + |
| 10 | +- Hardcoded secrets that could compromise authentication |
| 11 | +- Unauthenticated endpoints exposing sensitive user data |
| 12 | +- OAuth credentials exposed in repository |
| 13 | +- Production debug mode enabled |
| 14 | +- Weak default passwords |
| 15 | + |
| 16 | +--- |
| 17 | + |
| 18 | +## 🔴 CRITICAL ISSUE #1: Hardcoded Secrets |
| 19 | + |
| 20 | +**Severity:** CRITICAL |
| 21 | +**File:** `backend/app/__init__.py` |
| 22 | +**Lines:** 83-84 |
| 23 | + |
| 24 | +### Problem |
| 25 | +Default secret keys are hardcoded with weak values (`'rohank10'`). If environment variables are not set, the application will use these insecure defaults. |
| 26 | + |
| 27 | +### Code Location |
| 28 | +```python |
| 29 | +flask_app.config['SECRET_KEY'] = os.getenv('SECRET_KEY', 'rohank10') |
| 30 | +flask_app.config['JWT_SECRET_KEY'] = os.getenv('JWT_SECRET_KEY', 'rohank10') |
| 31 | +``` |
| 32 | + |
| 33 | +### Impact |
| 34 | +- **Authentication bypass:** Weak secrets can be brute-forced |
| 35 | +- **Session hijacking:** Compromised JWT tokens |
| 36 | +- **Data integrity:** Tampered session data |
| 37 | + |
| 38 | +### Fix Required |
| 39 | +1. Remove hardcoded defaults |
| 40 | +2. Require environment variables to be set |
| 41 | +3. Generate secure random secrets if not provided (with warning) |
| 42 | + |
| 43 | +### Recommended Fix |
| 44 | +```python |
| 45 | +# Option 1: Fail if not set (recommended for production) |
| 46 | +SECRET_KEY = os.getenv('SECRET_KEY') |
| 47 | +JWT_SECRET_KEY = os.getenv('JWT_SECRET_KEY') |
| 48 | +if not SECRET_KEY or not JWT_SECRET_KEY: |
| 49 | + raise ValueError("SECRET_KEY and JWT_SECRET_KEY must be set in environment") |
| 50 | + |
| 51 | +# Option 2: Generate with warning (development only) |
| 52 | +if not SECRET_KEY: |
| 53 | + import secrets |
| 54 | + SECRET_KEY = secrets.token_urlsafe(48) |
| 55 | + print("WARNING: Generated temporary SECRET_KEY. Set SECRET_KEY in environment for production!") |
| 56 | +``` |
| 57 | + |
| 58 | +--- |
| 59 | + |
| 60 | +## 🔴 CRITICAL ISSUE #2: Unauthenticated Debug Endpoint |
| 61 | + |
| 62 | +**Severity:** CRITICAL |
| 63 | +**File:** `backend/app/__init__.py` |
| 64 | +**Lines:** 356-432 |
| 65 | + |
| 66 | +### Problem |
| 67 | +The `/api/debug/user-role/<username>` endpoint is accessible without authentication and exposes sensitive user information including: |
| 68 | +- User ID |
| 69 | +- Email address |
| 70 | +- Role information |
| 71 | +- Internal database structure |
| 72 | + |
| 73 | +### Code Location |
| 74 | +```python |
| 75 | +@app.route('/api/debug/user-role/<username>', methods=['GET']) |
| 76 | +def debug_user_role(username): |
| 77 | + # No @require_auth decorator! |
| 78 | + # Exposes: id, username, email, role |
| 79 | +``` |
| 80 | + |
| 81 | +### Impact |
| 82 | +- **Information disclosure:** User emails and roles exposed |
| 83 | +- **User enumeration:** Attackers can check if usernames exist |
| 84 | +- **Privilege escalation:** Role information helps plan attacks |
| 85 | + |
| 86 | +### Fix Required |
| 87 | +1. Add `@require_auth` and `@require_role('admin')` decorators |
| 88 | +2. OR remove the endpoint entirely if not needed |
| 89 | +3. OR restrict to localhost/development only |
| 90 | + |
| 91 | +### Recommended Fix |
| 92 | +```python |
| 93 | +@app.route('/api/debug/user-role/<username>', methods=['GET']) |
| 94 | +@require_auth |
| 95 | +@require_role('admin') # Only admins can access |
| 96 | +@handle_errors |
| 97 | +def debug_user_role(username): |
| 98 | + # ... existing code ... |
| 99 | +``` |
| 100 | + |
| 101 | +**OR** Remove entirely if not needed in production. |
| 102 | + |
| 103 | +--- |
| 104 | + |
| 105 | +## 🔴 CRITICAL ISSUE #3: OAuth Secrets in Repository |
| 106 | + |
| 107 | +**Severity:** CRITICAL |
| 108 | +**File:** `backend/toolforge/toolforge_config.toml` |
| 109 | +**Lines:** 19-20 |
| 110 | + |
| 111 | +### Problem |
| 112 | +OAuth consumer key and secret are hardcoded in a configuration file that may be committed to version control. |
| 113 | + |
| 114 | +### Code Location |
| 115 | +```toml |
| 116 | +CONSUMER_KEY = "3f383c834a07a181723f1a1de566f7cf" |
| 117 | +CONSUMER_SECRET = "62c40e0fde2377613d1f82b9b7aabc9fe2a73b30" |
| 118 | +``` |
| 119 | + |
| 120 | +### Impact |
| 121 | +- **Account compromise:** If repository is public, OAuth credentials are exposed |
| 122 | +- **Unauthorized access:** Attackers can impersonate the application |
| 123 | +- **Data breach:** Access to user OAuth tokens |
| 124 | + |
| 125 | +### Fix Required |
| 126 | +1. **IMMEDIATELY:** Revoke and regenerate OAuth consumer credentials |
| 127 | +2. Move secrets to environment variables or secure secret management |
| 128 | +3. Add `toolforge_config.toml` to `.gitignore` if not already |
| 129 | +4. Use `.env.example` with placeholder values |
| 130 | + |
| 131 | +### Recommended Fix |
| 132 | +1. Revoke current OAuth consumer at: https://meta.wikimedia.org/wiki/Special:OAuthConsumerRegistration |
| 133 | +2. Create new OAuth consumer |
| 134 | +3. Store credentials in environment variables: |
| 135 | + ```bash |
| 136 | + export CONSUMER_KEY="new_key_here" |
| 137 | + export CONSUMER_SECRET="new_secret_here" |
| 138 | + ``` |
| 139 | +4. Update code to read from environment only |
| 140 | +5. Add to `.gitignore`: |
| 141 | + ``` |
| 142 | + backend/toolforge/toolforge_config.toml |
| 143 | + *.toml |
| 144 | + ``` |
| 145 | + |
| 146 | +--- |
| 147 | + |
| 148 | +## 🟠 CRITICAL ISSUE #4: Debug Mode in Production Code |
| 149 | + |
| 150 | +**Severity:** HIGH |
| 151 | +**Files:** |
| 152 | +- `backend/app/__init__.py:978` |
| 153 | +- `backend/main.py:24` |
| 154 | + |
| 155 | +### Problem |
| 156 | +Debug mode is hardcoded to `True` in the application startup code. This should be controlled by environment variables. |
| 157 | + |
| 158 | +### Code Location |
| 159 | +```python |
| 160 | +# backend/app/__init__.py:978 |
| 161 | +app.run( |
| 162 | + debug=True, # ⚠️ Hardcoded! |
| 163 | + host='0.0.0.0', |
| 164 | + port=5000 |
| 165 | +) |
| 166 | +``` |
| 167 | + |
| 168 | +### Impact |
| 169 | +- **Information leakage:** Detailed error pages expose internal structure |
| 170 | +- **Performance:** Auto-reload enabled in production |
| 171 | +- **Security:** Debug toolbar may expose sensitive data |
| 172 | + |
| 173 | +### Fix Required |
| 174 | +1. Use environment variable to control debug mode |
| 175 | +2. Default to `False` for production safety |
| 176 | +3. Only enable in development explicitly |
| 177 | + |
| 178 | +### Recommended Fix |
| 179 | +```python |
| 180 | +# backend/app/__init__.py |
| 181 | +if __name__ == '__main__': |
| 182 | + debug_mode = os.getenv('FLASK_DEBUG', 'False').lower() == 'true' |
| 183 | + app.run( |
| 184 | + debug=debug_mode, # Controlled by environment |
| 185 | + host='0.0.0.0', |
| 186 | + port=5000 |
| 187 | + ) |
| 188 | +``` |
| 189 | + |
| 190 | +--- |
| 191 | + |
| 192 | +## 🟠 CRITICAL ISSUE #5: Default Database Password |
| 193 | + |
| 194 | +**Severity:** MEDIUM-HIGH |
| 195 | +**File:** `backend/app/config.py` |
| 196 | +**Line:** 54 |
| 197 | + |
| 198 | +### Problem |
| 199 | +Default database connection string includes weak default password `'password'`. |
| 200 | + |
| 201 | +### Code Location |
| 202 | +```python |
| 203 | +SQLALCHEMY_DATABASE_URI = os.getenv( |
| 204 | + 'DATABASE_URL', |
| 205 | + 'mysql+pymysql://root:password@localhost/wikicontest' # ⚠️ Weak default |
| 206 | +) |
| 207 | +``` |
| 208 | + |
| 209 | +### Impact |
| 210 | +- **Database compromise:** If environment variable not set, uses weak password |
| 211 | +- **Data breach:** Unauthorized database access |
| 212 | + |
| 213 | +### Fix Required |
| 214 | +1. Remove default password (require DATABASE_URL to be set) |
| 215 | +2. OR use SQLite for development (no password needed) |
| 216 | +3. Document requirement in README |
| 217 | + |
| 218 | +### Recommended Fix |
| 219 | +```python |
| 220 | +# Option 1: Require DATABASE_URL (recommended) |
| 221 | +SQLALCHEMY_DATABASE_URI = os.getenv('DATABASE_URL') |
| 222 | +if not SQLALCHEMY_DATABASE_URI: |
| 223 | + raise ValueError("DATABASE_URL must be set in environment") |
| 224 | + |
| 225 | +# Option 2: Use SQLite for development |
| 226 | +SQLALCHEMY_DATABASE_URI = os.getenv( |
| 227 | + 'DATABASE_URL', |
| 228 | + 'sqlite:///wikicontest_dev.db' # No password needed |
| 229 | +) |
| 230 | +``` |
| 231 | + |
| 232 | +--- |
| 233 | + |
| 234 | +## 🟡 CRITICAL ISSUE #6: Missing Error Handler on Update Route |
| 235 | + |
| 236 | +**Severity:** MEDIUM |
| 237 | +**File:** `backend/app/routes/contest_routes.py` |
| 238 | +**Line:** 690 |
| 239 | + |
| 240 | +### Problem |
| 241 | +The `update_contest` route has `@require_auth` but is missing `@handle_errors` decorator, which could lead to inconsistent error handling. |
| 242 | + |
| 243 | +### Code Location |
| 244 | +```python |
| 245 | +@contest_bp.route("/<int:contest_id>", methods=["PUT"]) |
| 246 | +@require_auth |
| 247 | +# ⚠️ Missing @handle_errors |
| 248 | +def update_contest(contest_id): |
| 249 | +``` |
| 250 | + |
| 251 | +### Impact |
| 252 | +- **Inconsistent error responses:** Different error format than other routes |
| 253 | +- **Information leakage:** Unhandled exceptions may expose internal details |
| 254 | +- **Poor user experience:** Generic 500 errors instead of helpful messages |
| 255 | + |
| 256 | +### Fix Required |
| 257 | +Add `@handle_errors` decorator for consistency. |
| 258 | + |
| 259 | +### Recommended Fix |
| 260 | +```python |
| 261 | +@contest_bp.route("/<int:contest_id>", methods=["PUT"]) |
| 262 | +@require_auth |
| 263 | +@handle_errors # Add this |
| 264 | +def update_contest(contest_id): |
| 265 | +``` |
| 266 | + |
| 267 | +--- |
| 268 | + |
| 269 | +## Priority Action Items |
| 270 | + |
| 271 | +### Immediate (Before Any Production Deployment) |
| 272 | + |
| 273 | +1. ✅ **Fix hardcoded secrets** - Remove `'rohank10'` defaults |
| 274 | +2. ✅ **Secure debug endpoint** - Add authentication or remove |
| 275 | +3. ✅ **Revoke OAuth credentials** - Generate new ones, move to env vars |
| 276 | +4. ✅ **Disable debug mode** - Use environment variable |
| 277 | + |
| 278 | +### High Priority (Before Next Release) |
| 279 | + |
| 280 | +5. ✅ **Fix database password default** - Require DATABASE_URL |
| 281 | +6. ✅ **Add error handler** - Fix update_contest route |
| 282 | + |
| 283 | +### Additional Recommendations |
| 284 | + |
| 285 | +- Review all endpoints for missing authentication |
| 286 | +- Add security headers (HSTS, CSP, X-Frame-Options) |
| 287 | +- Implement rate limiting on authentication endpoints |
| 288 | +- Add input validation on all user inputs |
| 289 | +- Regular security audits |
| 290 | + |
| 291 | +--- |
| 292 | + |
| 293 | +## Testing Checklist |
| 294 | + |
| 295 | +After fixes are applied, verify: |
| 296 | + |
| 297 | +- [ ] No hardcoded secrets in codebase |
| 298 | +- [ ] All sensitive endpoints require authentication |
| 299 | +- [ ] Debug mode disabled in production |
| 300 | +- [ ] OAuth credentials in environment variables only |
| 301 | +- [ ] Database connection requires explicit configuration |
| 302 | +- [ ] All routes have consistent error handling |
| 303 | + |
| 304 | +--- |
| 305 | + |
| 306 | +## Notes |
| 307 | + |
| 308 | +- These issues were identified without affecting working features |
| 309 | +- All fixes should be backward compatible where possible |
| 310 | +- Test thoroughly after each fix |
| 311 | +- Consider security audit before production deployment |
0 commit comments