Conversation
This patch sets a 60-second timeout to all three HTTP request strategies (cURL, streams, and raw sockets) to mitigate the risk of Denial of Service (DoS) from requests that hang indefinitely on poor connections or tarpit attacks. Additionally, this patch moves the `verify_peer` option in `Post.php` out of the `http` array context and into the `ssl` array context where it belongs, ensuring SSL certificate verification functions as intended. The corresponding tests have been updated, and `stream_set_timeout` has been mocked in the `ReCaptcha\RequestMethod` namespace to ensure tests continue to pass. Co-authored-by: rowan-m <108052+rowan-m@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. |
Add type hints throughout. Add PHPStan configuration Enforce PHPStan on builds
…659096855075533595
rowan-m
deleted the
sentinel/add-timeouts-and-fix-ssl-context-11659096855075533595
branch
2 participants
🚨 Severity: MEDIUM
💡 Vulnerability: The Google reCAPTCHA PHP client makes outbound API requests (via cURL,
file_get_contents(), andfsockopen()) without setting explicit timeouts. Additionally, the fallbackPoststream method mistakenly setsverify_peer => truein thehttpcontext array rather than thesslcontext array, inadvertently causing it to be ignored by PHP.🎯 Impact: Requests to the reCAPTCHA API that hang indefinitely (e.g. from network instability or malicious "tarpit" behavior) will cause the requesting application thread to hang indefinitely, creating a Denial of Service (DoS) vulnerability. Furthermore, the misconfigured
verify_peeroption means stream requests may not correctly validate the peer certificate in some environments.🔧 Fix:
CURLOPT_TIMEOUTinCurlPost.php.httpcontext option inPost.php.verify_peeroption inPost.phpby moving it to thesslstream context.stream_set_timeout($handle, 60)inSocketPost.php.PostTest.phpandSocketPostTest.php.✅ Verification: The modifications have been tested via manual test scripts to confirm both
timeoutandverify_peerarray configurations operate correctly. The namespace-scoped mock forstream_set_timeoutguarantees test suite stability.PR created automatically by Jules for task 11659096855075533595 started by @rowan-m