Skip to content

fix(authx): avoid override secrets#5462

Merged
dwisiswant0 merged 6 commits intoprojectdiscovery:feat/authx/add-overwrite-fieldfrom
dwisiswant0:dwisiswant0/fix/avoid-overriding-secrets
Jul 30, 2024
Merged

fix(authx): avoid override secrets#5462
dwisiswant0 merged 6 commits intoprojectdiscovery:feat/authx/add-overwrite-fieldfrom
dwisiswant0:dwisiswant0/fix/avoid-overriding-secrets

Conversation

@dwisiswant0
Copy link
Copy Markdown
Member

@dwisiswant0 dwisiswant0 commented Jul 29, 2024

Proposed changes

Avoid overriding secrets.

Proof

Template:

# secret-file-requests.yaml
id: secret-file-requests

info:
  name: secret-file-requests
  author: dwisiswant0
  severity: info
  description: secret-file-requests
  tags: test

http:
  - raw:
    - | # foo:default
      GET / HTTP/1.1
      Host: {{Hostname}}
      X-Secret-Type: basicauth
      Authorization: Basic Zm9vOmRlZmF1bHQK

    - |
      GET / HTTP/1.1
      Host: {{Hostname}}
      X-Secret-Type: bearertoken
      Authorization: Bearer default

    - |
      GET / HTTP/1.1
      Host: {{Hostname}}
      X-Secret-Type: cookie
      Cookie: foo=default

    - |
      GET /?foo=default HTTP/1.1
      Host: {{Hostname}}
      X-Secret-Type: query

    - |
      GET / HTTP/1.1
      Host: {{Hostname}}
      X-Secret-Type: header
      foo: default

Secret File content:

# secret-file.yaml
static:
  - type: basicauth
    domains:
      - basicauth.oast.fun
    username: foo
    password: OVERRIDEN

  - type: bearertoken
    domains:
      - bearertoken.oast.fun
    token: OVERRIDEN

  - type: cookie
    domains:
      - cookie.oast.fun
    cookies:
      - key: foo
        value: OVERRIDEN
        # raw: "foo=OVERRIDEN"

  - type: query
    domains:
      - query.oast.fun
    params:
      - key: foo
        value: OVERRIDEN

  - type: header
    domains:
      - header.oast.fun
    headers:
      - key: foo
        value: OVERRIDEN

Command:

go run cmd/nuclei/main.go -duc -debug-req -sf secret-file.yaml -t secret-file-requests.yaml -u https://{basicauth,bearertoken,query,header,cookie}.oast.fun

Note

If you spot an OVERRIDEN value in the dumped HTTP request, it means that a hardcoded secret value has been overridden or replaced from the Secret File.

Checklist

  • Pull request is created against the dev branch
  • All checks passed (lint, unit/integration/regression tests etc.) with my changes
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)

Signed-off-by: Dwi Siswanto <git@dw1.io>
@dwisiswant0 dwisiswant0 changed the title fix(authx): avoid overriding Basic auth fix(authx): avoid overriding secrets Jul 29, 2024
Signed-off-by: Dwi Siswanto <git@dw1.io>
Signed-off-by: Dwi Siswanto <git@dw1.io>
Signed-off-by: Dwi Siswanto <git@dw1.io>
Signed-off-by: Dwi Siswanto <git@dw1.io>
@dwisiswant0 dwisiswant0 changed the title fix(authx): avoid overriding secrets fix(authx): avoid override secrets Jul 29, 2024
@dwisiswant0 dwisiswant0 marked this pull request as ready for review July 29, 2024 15:43
@dwisiswant0 dwisiswant0 requested a review from ehsandeep July 29, 2024 15:46
@ehsandeep ehsandeep requested a review from tarunKoyalwar July 29, 2024 16:00
Copy link
Copy Markdown
Member

@tarunKoyalwar tarunKoyalwar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i am not sure if this is right approach ,

from what i can understand user wants to avoid sending auth data when it is not required i.e HEAD , OPTIONS http methods .

apart from that i don't see a usecase where we don't want to override

we support many input sources ( proxify , burpsuite saved items being one of them ) . in proxy logs it is most likely that it will have credentials ( expired ones in most cases) and we would want to replace them in most /general scenarios, even for basicAuth replacing them does no harm

we can avoid sending them on OPTIONS , HEAD http request methods and for custom or non-general scenarios based on use case of user we can add more filters ( ex: maybe method or path based regex filter )

for conditions where exploit uses these headers we can add new variable in template field to skip-auth: true or something similar (if required)

cc: @ehsandeep

Template:

```yaml
  - |
    GET / HTTP/1.1
    Host: {{Hostname}}
    X-Secret-Type: header
    foo: default
```

If the secret file content is like this:

```yaml
  - type: header
    domains:
      - header.oast.fun
    overwrite: true
    headers:
      - key: foo
        value: OVERRIDEN
```

The request being sent is:

```
GET / HTTP/1.1
Host: header.oast.fun
User-Agent: Mozilla/5.0 (Kubuntu; Linux i686; rv:122.0) Gecko/20100101 Firefox/122.0
Connection: close
X-Secret-Type: header
foo: default
Foo: OVERRIDEN
Accept-Encoding: gzip
```

Signed-off-by: Dwi Siswanto <git@dw1.io>
@dwisiswant0 dwisiswant0 marked this pull request as draft July 29, 2024 20:32
@dwisiswant0
Copy link
Copy Markdown
Member Author

@tarunKoyalwar - the issue author concern is "[...] Authorization header either gets inserted or overridden".

[...] avoid sending them on OPTIONS , HEAD http request methods

But, I think the special conditions you mentioned are unnecessary since they don't really impact the server.

[...] we can add more filters ( ex: maybe method or path based regex filter )

Although I'm down with this.

I'm turning this into a draft and working on a workaround by adding an overwrite field. This seems like the best approach for this issue instead of adding a more sophisticated filter right now.

@dwisiswant0
Copy link
Copy Markdown
Member Author

UPDATE: Changing the base branch and removing the closing issue keywords.

@dwisiswant0 dwisiswant0 marked this pull request as ready for review July 29, 2024 20:59
@dwisiswant0 dwisiswant0 changed the base branch from dev to feat/authx/add-overwrite-field July 29, 2024 21:01
@CodeStuffBreakThings
Copy link
Copy Markdown

Hi all,
I just tested out the changes made by @dwisiswant0 and can confirm that when I specify the Authorization header in the raw HTTP request, that takes precedence over the Authorization header that gets inserted by the secret header. And if I remove the Authorization header from the raw HTTP request, the Authorization header from the secret file gets inserted as expected which satisfies my use case: In my scenario, I have 4 raw HTTP requests that need sent; one using a bogus credential pair for basic auth, and 3 that use the actual basic authentication credentials from the secret file.
Thank you so much for your help with this!

@dwisiswant0 dwisiswant0 merged commit 2245f2b into projectdiscovery:feat/authx/add-overwrite-field Jul 30, 2024
@dwisiswant0 dwisiswant0 deleted the dwisiswant0/fix/avoid-overriding-secrets branch July 30, 2024 11:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants