Skip to content

Conversation

@atharv02-git
Copy link

@atharv02-git atharv02-git commented Apr 30, 2025

Base Branch: app-attack-fixes

Note: Making a PR to thoth-tech/doubtfire-web so that this can be merged asap, and HardHat leads can retest this fix.

Description

This PR ensures that the internal nginx.conf inside doubtfire-web does not override the security headers (e.g., X-Frame-Options, Content-Security-Policy) that are now being enforced via the outer proxy-nginx.conf file in the doubtfire-deploy repository.

Note

Kindly go through the attached documentation first inorder to understand what this fix is about in detail and how it can be tested.

What was changed:

  • Commented out redundant security headers from doubtfire-web/nginx.conf to prevent conflict or override with headers applied at the reverse proxy layer (proxy-nginx.conf).
  • This avoids duplication and ensures centralized management of security headers at the proxy level for consistency across services.

Fixes # (Header override issues caused by multiple NGINX layers)

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • This change requires a documentation update

How Has This Been Tested?

  • Validated that headers set in proxy-nginx.conf (doubtfire-deploy) reflect in browser response
  • Confirmed no duplication or override from doubtfire-web/nginx.conf
  • Ensured static files are still served correctly via inner NGINX
  • Yet to test Clickjacking Prevention in a Malicious <Iframe> Setup as listed in the report.

Testing Checklist:

  • Tested in latest Chrome
  • Needs to be tested inside a dedicated environment like kali linux inside a virtual box.

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings

@atharv02-git atharv02-git changed the title Removed CSP from nginx.conf to avoid conflicts with the headers in pr… app-attack-fixes/clickjacking-vulnerability May 5, 2025
@aNebula aNebula self-requested a review May 5, 2025 06:56
@aNebula aNebula merged commit f171d39 into thoth-tech:app-attack-fixes May 5, 2025
@DarrylO21
Copy link

Hi @atharv02-git, I have reviewed the removal of the conflicting Content-Security-Policy and Feature-Policy headers in this section, and the rationale makes sense. Commenting them out to avoid overriding the security headers defined in proxy-nginx.conf is a clean and effective approach. This change helps maintain a centralized and conflict-free security configuration.

Best regards,
Darryl
PT and SCR Senior Lead, AppAttack

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