webhooksite / webhook.site

⚓️ Easily test HTTP webhooks with this handy tool that displays requests instantly.
https://webhook.site
Other
5.32k stars 416 forks source link

Passing in `webhooksite_session` cookie causes entire cookie header not to display #161

Closed srithon closed 4 months ago

srithon commented 4 months ago

For context, I was trying to figure out whether the browser sent cookies in the following example, from codesandbox.io; I had seen online that CORS policies did not apply to script tags and wanted to see it for myself:

<script
      src="https://webhook.site/b37a80bd-bf73-424b-9112-72ff1a349cfd"
      type="text/javascript"
    ></script>

However, after doing some experimentation, I realized that even when navigating to the Webhook URL directly from the browser (in a new tab), the Cookie header doesn't display in the UI, despite definitely being sent in the request. Further experiments with curl showed that the Cookie header would show up in the UI as long as webhooksite_session wasn't one of the cookies. The other cookie, XSRF-TOKEN, didn't have the same problem. I tried going through the code to see where/why this was being done, but couldn't figure it out for the life of me. Then, after looking at the project's README I realized that the codebase for the cloud version is separate, so there was no point trying to debug it 🤦. With that said, once you figure out the issue, if possible, could you briefly explain what was causing this behavior, just for personal curiosity reasons?

fredsted commented 4 months ago

It's to avoid leaking the authentication cookies of Webhook.site users, although I would say it could be done in a more elegant way.

srithon commented 4 months ago

It's to avoid leaking the authentication cookies of Webhook.site users, although I would say it could be done in a more elegant way.

Got it, got it, thanks, I didn't consider that webhook URLs are meant to be publicly shareable. I completely understand that this isn't a priority, but it would be cool if instead of dropping the Cookie header entirely, you instead replaced the webhooksite_session value with "\<redacted>". I attempted to draft some code for this but as I'm not familiar with PHP (and the cloud codebase may differ here), it may be incorrect:

./app/Storage/Request.php
@@ -1,9 +1,11 @@
 use Carbon\Carbon;
 use Illuminate\Http\Request as HttpRequest;
 use Ramsey\Uuid\Uuid;

+use Symfony\Component\HttpFoundation\HeaderUtils;
+
 class Request extends Entity
 {
     /**
      * @param $tokenId
      * @param null $requestId
@@ -22,10 +24,18 @@
      * @param HttpRequest $httpRequest
      * @return Request
      */
     public static function createFromRequest(HttpRequest $httpRequest)
     {
+        $cookies = $httpRequest->cookies();
+        $auth_token_name = 'webhooksite_session';
+        if ($cookies->get($auth_token_name)) {
+            $cookies->set($auth_token_name, '<redacted>');
+            $redactedCookiesVal = HeaderUtils::toString($cookies->all(), '; ');
+            $httpRequest->headers->set('cookie', $redactedCookiesVal);
+        }
+
         $request = new self([
             'uuid' => Uuid::uuid4()->toString(),
             'token_id' => $httpRequest->tokenId,
             'ip' => $httpRequest->ip(),
             'hostname' => $httpRequest->getHost(),

While this approach does have the downside of normalizing the HTTP header value (removing whitespace and potentially also normalizing the case), I would still prefer the behavior over just dropping the Cookie header entirely. However, since this is an issue most people wouldn't be concerned with, I understand if you'd like to avoid the additional complexity/capacity for bugs altogether.