Security: Strengthen CSRF protection with enhanced validation#118
Open
lancejames221b wants to merge 1 commit intoi2p:masterfrom
Open
Security: Strengthen CSRF protection with enhanced validation#118lancejames221b wants to merge 1 commit intoi2p:masterfrom
lancejames221b wants to merge 1 commit intoi2p:masterfrom
Conversation
Critical security fix addressing CWE-352 (Cross-Site Request Forgery): Changes made: - Remove dangerous CSRF bypass when password authentication is enabled - Implement multi-layer CSRF token validation with fallback support - Add constant-time string comparison to prevent timing attacks - Enhance nonce generation with more secure token creation (256-bit) - Provide session-specific token framework (foundation for future enhancement) - Maintain backward compatibility with existing token system Security improvements: - All forms now require CSRF validation regardless of authentication mode - Stronger token generation using multiple cryptographic random values - Defense against timing attacks on token comparison - Proper error messages for invalid token attempts - Future-ready architecture for session-isolated tokens This fix prevents unauthorized state-changing requests by ensuring all form submissions include valid CSRF tokens, eliminating the authentication bypass vulnerability and strengthening overall CSRF protection. Author: Lance James, Unit 221B, Inc
Contributor
|
Agreed we shouldn't disable the token check if password is enabled. I'll apply that part of it. The rest of it I'm skeptical. 64 bit tokens seems plenty, and the timing attack vector seems implausible. If we do want something longer, we should generate it with one call to nextBytes(byte[]), not 4 calls to nextLong() with 4 string appends. There's also no need for 'backward compatibility' code, this is just a string stuck in every web form on the fly. |
github-actions bot
pushed a commit
to eyedeekay/i2p.i2p
that referenced
this pull request
Aug 22, 2025
Adapted from Github PR i2p#118 thanks Lance James
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Security Fix: CSRF Protection Bypass
Vulnerability Summary
MEDIUM SEVERITY - Fixes CSRF protection bypass that could allow cross-site request forgery attacks when password authentication is enabled.
CVSS Score
Medium (6.1) - Cross-Site Request Forgery
Vulnerability Details
apps/routerconsole/java/src/net/i2p/router/web/FormHandler.javaChanges Made
Security Improvements
Technical Details
Testing
Files Modified
apps/routerconsole/java/src/net/i2p/router/web/FormHandler.javaAuthor: Lance James, Unit 221B, Inc - aka 0x90