AjaxSearch - Security Issue, Bug 1.8.1, Non-Ajax

64 Posts

Katzenfutter Reply #1, 17 years, 7 months ago

If you defined a landing page which would provide you with the search results, you could easily browse through the result set using an offset.
This offset might be an integer, a string or whatever. I´m seeing a security issue here.
This offset, called "AS_offset", used as a negative integer makes modX react with a Parse Error as seen here:
http://www.modxcms.com/search-results.html&FSF_offset=-70&FSF_search=modx
You could also make Apache (or what ever you are using) using high CPU-load using a string as an offset, seen here:
http://www.modxcms.com/search-results.html&FSF_offset=bla&FSF_search=modx

This issue might be solved very easily by checking the $_GET-Variable of AS_offset.
You can find this code near line 300 in "ajaxSearch.class.inc.php".

I fixed it on my own like this (might be a bit naive, but works wink

):

//check for paging offset
    $offset_val = 0;
    if(isset($_GET['AS_offset']))
    {
    	$offset_val = intval($_GET['AS_offset']);
    }
    
    if($offset_val >= 0)
    	$this->offset = $offset_val;

Edit:
If the offset exceeds the number of documents retrieved, modx crashes too... this needs to be fixed.

Greetings from Germany

$cd /pub
$more beer

1,717 Posts

coroico Reply #2, 17 years, 7 months ago

Many thanks Katzenfutter. Your remarks are very helpfull to improve the security of AjaxSearch.

I will include this update in the next release AS 1.8.2. and register this issue as AJAXSEARCH-25

Regarding:

If the offset exceeds the number of documents retrieved, modx crashes too... this needs to be fixed.

I will come back to you with a solution.

Greetings from the other side of the Rhine River wink

1,717 Posts

coroico Reply #3, 17 years, 7 months ago

If the offset exceeds the number of documents retrieved, modx crashes too... this needs to be fixed.

Around the line 535 of classes/ajaxSearch.class.inc.php file, just after the line:

$numResultPages = ceil($nbrs/$grabMax);

adds:

      $maxOffset = ($numResultPages-1) * $grabMax;
      $this->offset = ($this->offset > $maxOffset) ? $maxOffset : $this->offset;

With this fix, you will get the last page if the offset exceeds the number of documents retrieved
Issue registered as AJAXSEARCH-26

1,717 Posts

coroico Reply #4, 17 years, 7 months ago

AJAXSEARCH-26 fixed with the version 1.8.2 see this demo page
Here an offset of 40 (&AS_offset=40) give the third page and avoid a crash of MODx

1,717 Posts

coroico Reply #5, 17 years, 7 months ago

AJAXSEARCH-25 - In the version 1.8.2 I have checked the $_GET Variable of AS_offset as follow:

    //check for paging offset
    $this->offset = (isset($_GET['AS_offset'])) ? intval($_GET['AS_offset']) : 0;
    $this->offset = ($this->offset >0) ? $this->offset : 0;

These three lines could replaced the lines 302 & 303 of the version 1.8.1:

    //check for paging offset
    $this->offset = (isset($_GET['AS_offset'])) ? $_GET['AS_offset'] : 0;

64 Posts

Katzenfutter Reply #6, 17 years, 7 months ago

Thanks a lot mate for reacting and responding that soon!

Cheers!

$cd /pub
$more beer