Skip to content

Conversation

@bitterpanda63
Copy link
Member

@bitterpanda63 bitterpanda63 commented Dec 15, 2025

Summary by Aikido

⚡ Enhancements

  • Added domain-based outbound blocking and service config integration
  • Reported and blocked hostnames before DNS resolution in socket sink

🔧 Refactors

  • Removed hostname caching from SSRF vulnerability scan path

@bitterpanda63 bitterpanda63 changed the title draft: add outbound blocking add outbound blocking Dec 23, 2025
@bitterpanda63 bitterpanda63 marked this pull request as ready for review December 23, 2025 00:02
return self.bypassed_ips.has(ip)

def update_domains(self, domains):
self.domains = {domain["hostname"]: domain["mode"] for domain in domains}
Copy link

@aikido-pr-checks aikido-pr-checks bot Dec 23, 2025

Choose a reason for hiding this comment

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

update_domains mutates self.domains (a shared dict) without synchronization; concurrent readers may observe partial updates or race conditions.

Details

✨ AI Reasoning
​ServiceConfig gained mutable fields domains and block_new_outgoing_requests that are written by update_service_config and read by sinks. The methods update_domains and set_block_new_outgoing_requests mutate shared ServiceConfig state without any synchronization or atomic swap, introducing a data race between writer and readers.

🔧 How do I fix it?
Use locks, concurrent collections, or atomic operations when accessing shared mutable state. Avoid modifying collections during iteration. Use proper synchronization primitives like mutex, lock, or thread-safe data structures.

More info - Comment @AikidoSec feedback: [FEEDBACK] to get better review comments in the future.

if not cache:
return

cache.hostnames.add(hostname, port)
Copy link

@aikido-pr-checks aikido-pr-checks bot Dec 23, 2025

Choose a reason for hiding this comment

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

Modifies shared collection cache.hostnames (cache.hostnames.add) without synchronization; this mutates shared state and can cause races or iteration errors.

Details

✨ AI Reasoning
​The new function report_and_check_hostname retrieves a shared thread-local/global cache via get_cache() and calls cache.hostnames.add(hostname, port) (line 18). This is a mutation of shared mutable state. There's no locking or atomic API shown, and other threads may concurrently access or iterate the same hostnames collection, risking race conditions or runtime errors (e.g., mutation during iteration). The change moved hostname registration earlier (it used to be added in vulnerabilities.init but that code was removed), so this PR introduced the new mutation point.

🔧 How do I fix it?
Use locks, concurrent collections, or atomic operations when accessing shared mutable state. Avoid modifying collections during iteration. Use proper synchronization primitives like mutex, lock, or thread-safe data structures.

More info - Comment @AikidoSec feedback: [FEEDBACK] to get better review comments in the future.

@bitterpanda63 bitterpanda63 merged commit 95f189e into main Dec 23, 2025
82 of 83 checks passed
@bitterpanda63 bitterpanda63 deleted the add-outbound-blocking branch December 23, 2025 13:10
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