digininja / DVWA

Damn Vulnerable Web Application (DVWA)
GNU General Public License v3.0
10.36k stars 3.65k forks source link

Deploying DVWA behind a reverse proxy with a prefix #588

Closed adb014 closed 1 year ago

adb014 commented 1 year ago

I'm deploying DVWA behind a reverse proxy with an htdigest to allow a limited number of people access to it to play around. My problem is that my nginx proxy rewrites the prefix for DVWA and HTTP_X_FORWARDED_PREFIX is not taken into account during the redirects, this messes up the redirected page, for example when changing the security level. A simple patch

diff -u DVWA-2.3/dvwa/includes/dvwaPage.inc.php.orig DVWA-2.3/dvwa/includes/dvwaPage.inc.php
--- DVWA-2.3/dvwa/includes/dvwaPage.inc.php.orig    2023-10-24 20:52:21.853896655 +0200
+++ DVWA-2.3/dvwa/includes/dvwaPage.inc.php 2023-10-24 20:53:21.023468344 +0200
@@ -113,7 +113,7 @@

 function dvwaPageReload() {
-   dvwaRedirect( $_SERVER[ 'PHP_SELF' ] );
+   dvwaRedirect( $_SERVER[ 'HTTP_X_FORWARDED_PREFIX' ] . $_SERVER[ 'PHP_SELF' ] );
 }

 function dvwaCurrentUser() {

fixes this. As $_SERVER['HTTP_X_FORWARDED_PREFIX'] will not be set if DVWA is not behind a proxy, this change won't impact DVWA in normal use.

digininja commented 1 year ago

When HTTP_X_FORWARDED_PREFIX isn't in the SERVER hash, PHP will throw a warning about accessing an undefined key. You'll need to add some additional checking and only add it if it exists.

adb014 commented 1 year ago

Will you accept a patch like that ? If so I'll propose something will the additional checking, something like

if  ( array_key_exists(  'HTTP_X_FORWARDED_PREFIX' , $_SERVER ))
    dvwaRedirect( $_SERVER[ 'HTTP_X_FORWARDED_PREFIX' ] . $_SERVER[ 'PHP_SELF' ] );
else
    dvwaRedirect( $_SERVER[ 'PHP_SELF' ] );

Not sure if I can be bothered to clone the repo and do a pull request for such a simple change though

digininja commented 1 year ago

I would go with if array key exists but I'm happy with that.

If you fork the repo you can edit it in GitHub and do the pr from there.

On Wed, 25 Oct 2023, 10:58 adb014, @.***> wrote:

Will you accept a patch like that ? If so I'll propose something will the additional checking, something like

if ( empty( $_SERVER[ 'HTTP_X_FORWARDED_PREFIX' ] ) dvwa( $_SERVER[ 'PHP_SELF' ] ); else dvwaRedirect( $_SERVER[ 'HTTP_X_FORWARDED_PREFIX' ] . $_SERVER[ 'PHP_SELF' ] );

Not sure if I can be bothered to clone the repo and do a pull request for such a simple change though

— Reply to this email directly, view it on GitHub https://github.com/digininja/DVWA/issues/588#issuecomment-1778919254, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAA4SWIL44OZKMHZMVN25STYBDPF5AVCNFSM6AAAAAA6O6RH7WVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTONZYHEYTSMRVGQ . You are receiving this because you commented.Message ID: @.***>

RAD50 commented 1 year ago

I'm deploying DVWA behind a reverse proxy with an htdigest to allow a limited number of people access to it to play around. My problem is that my nginx proxy rewrites the prefix for DVWA and HTTP_X_FORWARDED_PREFIX is not taken into account during the redirects, this messes up the redirected page, for example when changing the security level. A simple patch

diff -u DVWA-2.3/dvwa/includes/dvwaPage.inc.php.orig DVWA-2.3/dvwa/includes/dvwaPage.inc.php
--- DVWA-2.3/dvwa/includes/dvwaPage.inc.php.orig  2023-10-24 20:52:21.853896655 +0200
+++ DVWA-2.3/dvwa/includes/dvwaPage.inc.php   2023-10-24 20:53:21.023468344 +0200
@@ -113,7 +113,7 @@

 function dvwaPageReload() {
- dvwaRedirect( $_SERVER[ 'PHP_SELF' ] );
+ dvwaRedirect( $_SERVER[ 'HTTP_X_FORWARDED_PREFIX' ] . $_SERVER[ 'PHP_SELF' ] );
 }

 function dvwaCurrentUser() {

fixes this. As $_SERVER['HTTP_X_FORWARDED_PREFIX'] will not be set if DVWA is not behind a proxy, this change won't impact DVWA in normal use.

can you make a tutorial on how to set it on some virtual VM ? so I can access it anywhere? like on digitalocean ......etc ?

digininja commented 1 year ago

Please don't hijack other issues with your own questions, start your own.

adb014 commented 1 year ago

Well the hijack made me remember this minor patch. Pull request in https://github.com/digininja/DVWA/pull/593.

@RAD50 I'll answer in another issue if you create it

adb014 commented 1 year ago

Pull request accepted, closing