Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Vulcan subdomain takeover #790

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

seilagamo
Copy link
Contributor

@seilagamo seilagamo commented Jan 22, 2025

It's necessary to implement a check to detect potential subdomain takeovers.

The flow that generates the vulnerability is:

  • Person X generates a DNS record with a subdomain of ours pointing to an AWS resource.
  • Person X decides to delete the resource for whatever reason without deleting the DNS record.
  • An attacker identifies that there is a DNS record with a subdomain of ours pointing to an AWS resource that does not exist.
  • The attacker decides to register an AWS resource with the identifier pointed to by the DNS record.

@seilagamo seilagamo marked this pull request as draft January 22, 2025 15:38
@seilagamo seilagamo changed the base branch from master to checkshttp January 23, 2025 08:03
@seilagamo seilagamo force-pushed the vulcan-subdomain-takeover branch from 2443094 to edef026 Compare January 28, 2025 08:35
@seilagamo seilagamo marked this pull request as ready for review January 29, 2025 15:42
@seilagamo seilagamo requested review from jesusfcr, jroimartin and danfaizer and removed request for jesusfcr January 29, 2025 15:43
Copy link
Contributor

@jesusfcr jesusfcr left a comment

Choose a reason for hiding this comment

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

Great job.

I added some minor comments, but the main concern is that it doesn't take into consideration multi region.

Apart from that I think the name of the check is too generic for the current implementation. One posibility could be to change to vulcan-aws-subdomain-takeover. Other posibility could be to include this functionality in the current vulcan-aws-alerts check.

Apart from that, I probably base the PR on master and not to checkshttp to prevent blockings.

cmd/vulcan-govulncheck/Dockerfile Outdated Show resolved Hide resolved
cmd/vulcan-subdomain-takeover/Dockerfile Outdated Show resolved Hide resolved
cmd/vulcan-subdomain-takeover/main.go Outdated Show resolved Hide resolved
cmd/vulcan-subdomain-takeover/main.go Outdated Show resolved Hide resolved
cmd/vulcan-subdomain-takeover/main.go Outdated Show resolved Hide resolved
cmd/vulcan-subdomain-takeover/main.go Outdated Show resolved Hide resolved
@seilagamo seilagamo force-pushed the vulcan-subdomain-takeover branch from d831051 to 864e8c0 Compare February 4, 2025 10:44
@seilagamo seilagamo changed the base branch from checkshttp to update-govulncheck February 4, 2025 10:44
@seilagamo seilagamo force-pushed the vulcan-subdomain-takeover branch from 6ff6e69 to 613d192 Compare February 5, 2025 15:42
Base automatically changed from update-govulncheck to master February 6, 2025 11:25
@seilagamo seilagamo force-pushed the vulcan-subdomain-takeover branch from a1d34d3 to 2c4584a Compare February 6, 2025 11:30
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.

2 participants