🔒 Fix global hostname verification vulnerability#30
Conversation
Removed the `setDefaultHostnameVerifier` call in `BaseJsoupSource` which was globally disabling hostname verification when `ignoreSslErrors` was enabled. Refactored `NovelFireSource` to use `OkHttpClient` for search queries to avoid reliance on the vulnerable Jsoup connection path. Added a regression test to verify that the global verifier is no longer compromised. Co-authored-by: Aatricks <113598245+Aatricks@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
This PR addresses a critical security vulnerability where
HttpsURLConnection.setDefaultHostnameVerifierwas being set to trust all hostnames globally when the "Ignore SSL Errors" preference was enabled. This exposed the entire application to Man-in-the-Middle (MitM) attacks.Changes:
app/src/main/java/io/aatricks/novelscraper/data/repository/source/BaseJsoupSource.kt: Removed the lines that set the global default hostname verifier to always returntrue. The per-connectionsslSocketFactorysetting is preserved to allow trusting self-signed certificates (if desired) but without compromising hostname verification.app/src/main/java/io/aatricks/novelscraper/data/repository/source/NovelFireSource.kt: RefactoredsearchNovelsto use the injectedokHttpClientdirectly instead of theconnect()helper method. This improves safety and leverages the modern HTTP client.app/src/test/java/io/aatricks/novelscraper/data/repository/source/BaseJsoupSourceSecurityTest.kt: Added a new test file to verify that the fix works and that the global hostname verifier remains secure (rejects invalid hostnames) even when the vulnerable code path is triggered.Risk:
PR created automatically by Jules for task 2708113590480199099 started by @Aatricks