Gallery 1.5 and XSS trigger + work around

New Member Joined: Aug 16, 2003 Posts: 2

W/the nuke embedded Gallery 1.5 I noticed a trigger on the XSS check when I went to change the Highlighted picture for an album. Looking, at at first thinking w/Highlight it was another Santy block, I commented that out w/no success..

Basically, the URL is module.php?cmd= that triggers the XSS. since I didn't want to disable the XSS protect, simply changed the order or the URL, to some=x&cmd= would bypass the filter I edited the view_album.php file at line 966 to the following:

Code:

                  showChoice(sprintf(_("Highlight %s"),$label), 'do_command.php', array('index' => $i,'cmd' => 'highlight'));

from the previous

Quote:

showChoice(sprintf(_("Highlight %s"),$label), 'do_command.php', array('cmd' => 'highlight','index' => $i));

Changing the order of the gets was all that was required, maybe this may help others..

Posted: Fri Sep 09, 2005 10:54 am

Thank you. I tried your patch with Gallery 1.4 also and it works fine and it means you can leave the nukesentinel.php file alone. That's great. Especially since the updates to Sentinel leave the old XSS edits in place so you'd have to change those again to make highlighting work with Sentinel.

Posted: Sun Sep 11, 2005 10:57 pm

If we were to change the filter it could open your site to attack. At the same time we do not want to have conflicts with other modules/addons so we will look and see if there is a simple solution for this.

Posted: Mon Sep 12, 2005 7:40 am

For what it's worth, a user reported to be being banned when he went to "hide" a photo in Gallery last night (9/11). I unbanned him and told him not to use the function until we looked into it but I think it does the same thing as the highlight function did. I'm just bringing this to your attention in that Gallery may have some other functions that cause users to get banned and it's my intention to look into it to see. There may be some way to globally search and replace the problems within Gallery code but then of course we'd have problems replicating it when Gallery is upgraded.

Thanks for looking into it.

Later: Looked at the code in Gallery view_album.php. Down around line 1050 of version 1.4.4 there is the following code for showing and hiding a photo:

Code:

                showChoice(sprintf(_("Show %s"), $label), "do_command.php", array("index" => $i, "cmd" => "show", ));


            } else {


               showChoice(sprintf(_("Hide %s"), $label), "do_command.php", array( "index" => $i, "cmd" => "hide",));


            }

Actually, this is the code after revision. As per Viper's suggestion at the start of this thread I just flipped around the order of index and cmd in the array. Tests out okay on my system, hides the photo and doesn't upset Sentinel.

Posted: Mon Sep 12, 2005 4:36 pm

BobMarion wrote:

If we were to change the filter it could open your site to attack. At the same time we do not want to have conflicts with other modules/addons so we will look and see if there is a simple solution for this.

Ya I didn't really want to play around removing checks out of Sentinel, so I figured it would be easier to mod Gallery itself

Posted: Fri Dec 09, 2005 10:48 am

Sorry to resurface a "resolved" issue but it resurfaced itself.

As a result of a recent upgrade to Nuke 7.6 (from 7.4) along with patched 3.1 and Sentinel 2.4.2, a number of Gallery issues re-emerged. To try to resolve the issues I upgraded Gallery to the latest 1.x series release = 1.5.1. This just caused more problems. One, relating to code in mainfile is addressed here:

http://www.ravenphpscripts.com/postt6361.html

But a more vexing set arose because the way Gallery passes "cmd" in the query string has changed significantly in this release and I don't believe Viperlord's fixes will work for it anymore. Specifically, the strings "cmd=highlight", "cmd="hide" and "cmd=show" which cause NukeSentinel to ban the IP are no longer done in the view_album.php program in Gallery but are rather "buried" somewhere else, I think in an object somewhere but I haven't been successful in finding them.

Because I didn't want to keep having to "hack" Gallery everytime there was an upgrade, I turned my attention to the filter code in Sentinel and came up with this. It works on my site but I would appreciate feedback in case I have opened up some other holes. I have a few questions too after the code.

Code:

  // Check for XSS attack


  // I believe we look for http regardless of module and block it unconditionally so I separated this out


  // less darn parentheses to contend with in the rest of the logic


   if (eregi("http\:\/\/", $name)) {


           block_ip($blocker_row);


       }


  if ($name == "Gallery") {


     // now deal with command string alone 


     if (stristr($nsnst_const['query_string'], "cmd=") AND !stristr($nsnst_const['query_string'], "&cmd") 


             AND  (!stristr($nsnst_const['query_string'], "cmd=highlight") AND 


           !stristr($nsnst_const['query_string'], "cmd=hide") AND  


            !stristr($nsnst_const['query_string'], "cmd=show"))) 


            {      


                   block_ip($blocker_row);


          }


     if (((stristr($nsnst_const['query_string'], "exec") AND !stristr($nsnst_const['query_string'], "execu")) OR


  (stristr($nsnst_const['query_string'],"concat") AND


  !stristr($nsnst_const['query_string'], "../")))) {


       block_ip($blocker_row);


       }


    }  // end of that its gallery


else { 


    // not gallery


  if ((stristr($nsnst_const['query_string'], "cmd=") AND !stristr($nsnst_const['query_string'], "&cmd")) OR 


  (stristr($nsnst_const['query_string'], "exec") AND !stristr($nsnst_const['query_string'], "execu")) OR


  stristr($nsnst_const['query_string'],"concat") AND


  !stristr($nsnst_const['query_string'], "../")) {


    block_ip($blocker_row);


   } 


  }  // end that its not gallery


}

I put some comments in and raise these questions.

1. Shouldn't the check for the string "http://" be done in all cases and wouldn't it be wise to just put it first, maybe even before the checks for hacks to news, reviews, forums?

2. I did my checks for Gallery in "pieces" cause I have having trouble getting the right number of parentheses to match. My low IQ means that when I go above about 3 levels of parentheses I start to get confused and I find it easier to "segment" things.

3. I'm not sure what the history of hacks using "cmd" is but looking at the code, I'm just wondering, if we want to screen out "cmd=" but we exclude situations where there is the string "&cmd" in the query string, couldn't a hacker get around the check by just placing an "innocuous" "&cmd" somewhere later in the string?

I'd really appreciate any feedback or suggestions on my approach. I tried to limit the "exceptions" I made to the three situations that I know of where Gallery passes a "cmd=" string legitimately (hide, show, and highlight) but there may be more that could easily be added.

Posted: Fri Dec 09, 2005 4:22 pm

Driving around this afternoon I was thinking more about this. I'm hoping that some of the Sentinel experts can look at it and maybe address these questions too;

1. It looks to me like the intent of these filters is to stop someone from executing an "external" command, let's say a Unix command. Is that right? Or am I misreading it? Here's the actual full request that Gallery passes in (edited a bit):

Quote:

Query String: mysite.org/xxx/xxx/cms/modules.php?cmd=show&index=2&set_albumName=album07&type=popup&op=modload&name=Gallery&file=index&include=do_command.php

What Viperlord originally did was flip the position of cmd=show and index=x, before it was passed from Gallery, I believe and I've always been a bit puzzled about why it worked. I think that Gallery is using the "cmd" for its own internal purposes, it gets passed back to do_command.php, rather than executing a system command. So, it's probably unfortunate that Gallery is using this particular ambiguous string. But of course my understanding could be way off base.

2. I'm not sure if cmd= has to be the first sequence in a request string to execute a system command, or maybe this is not what this whole check is about. If a cmd=do_something_nefarious can be executed anywhere in the request string then maybe my added checks for Gallery should substring the first x characters in the request string and only do the exception where it is present there. Also could there be two cmd= sequences in a string and if so would the system accept them both, or the first or the last?

Sorry for all the questions but I'm trying to understand and contribute. Also I'm trying to piggyback on existing threads, even if Sentinel non-2.4.2 is a bit outdated, rather than having Gallery and Sentinel threads all over.