Page 1 of 1

SQL injection problem

Posted: Thu Jan 20, 2011 1:39 pm
by aleborg
I am trying to do a classfield extension to punbb but have problems to do it secure enough for sql injections.

Code: Select all

$search_string = "";
if($w != ""){
    $words = explode(",", $w);
    $words = trimarray($words);
    //$words = array_merge(array(""), $words);
    $string_of_words = implode(" ", $words);
    $search_string = " AND MATCH (a.body, a.header)
    AGAINST (
    '" . $string_of_words . "'
    IN BOOLEAN
    MODE
    )";
}

if($cat != "0"){
    $search_string .= " AND a.category = " . $cat;
}

if($district != "0"){
    $search_string .= " AND a.lan = " . $district;
}

if($type != "2"){
    $search_string .= " AND a.ad_type = " . $type;
}

////////////////////////////PAGING

// Fetch ads count
$result = $db->query('SELECT COUNT(id) FROM '.$db->prefix.'ads AS a WHERE a.active = 1 '.$search_string.'') or die(mysql_error());
$num_ads = $db->result($result);

// Determine the user offset (based on $_GET['p'])
$num_pages = ceil($num_ads / $posts_per_page);
$p = (!isset($_GET['p']) || $_GET['p'] <= 1 || $_GET['p'] > $num_pages) ? 1 : $_GET['p'];
$start_from = $posts_per_page * ($p - 1);
$paging_links = $lang_common['Pages'].': '.paginate($num_pages, $p, 'viewads.php?w='.urlencode($w).'&cat='.$cat.'&dist='.$district.'&type='.$type.'&search=Submit');


define('PUN_ALLOW_INDEX', 1);
require PUN_ROOT.'header.php';

?>
<?php 
    if($pun_user['is_guest']){
        echo "Login to add an classfield";
    }
    else{
        echo "<a href='newad.php'>Add classfield</a>";
    }
?>
<br /><br />
<div class="blockform">
    <h2><span>Search for an ad</span></h2>
    <div class="box">
        <form id="adlist" method="get" action="viewads.php">
        <div class="inform">

            <fieldset>
                <legend>Search</legend>
                <div class="infldset">
                    <label class="conl">Search word<br /><input type="text" name="w" value="<?php echo $w != '' ? $w : ''; ?>" size="25" maxlength="25" /><br /></label>
                    <label class="conl">Category
                    <br /><select name="cat">
                        <option value="0" selected="selected">All categorys</option>
                        <?php
                        $query = ('SELECT id, description FROM '.$db->prefix.'ad_categories ORDER BY description');
                        $result = $db->query($query) or die (mysql_error());
                        while ($categories = $db->fetch_assoc($result)):?>
                         <option value="<?php echo $categories['id']; ?>" <?php echo $cat == $categories['id'] ? "selected='selected'" : ""; ?>><?php echo utf8_decode($categories['description']); ?></option>
                        <?php endwhile; ?>
                    </select>
                    <br /></label>
                    <label class="conl">Ort
                    <br /><select name="dist">
                        <option value="0" selected="selected">Hela Sverige</option>
                        <?php
                        $query = ('SELECT id, description FROM '.$db->prefix.'district ORDER BY description');
                        $result = $db->query($query) or die (mysql_error());
                        while ($districts = $db->fetch_assoc($result)):?>
                         <option value="<?php echo $districts['id']; ?>" <?php echo $district == $districts['id'] ? "selected='selected'" : ""; ?>><?php echo utf8_decode($districts['description']); ?></option>
                        <?php endwhile; ?>
                    </select>
                    <br /></label>
                    <label class="conl">Ad type
                    <br /><select name="type">
                        <option value="2" <?php echo $type == 2 ? "selected='selected'" : ""; ?>>All</option>
                        <option value="0" <?php echo $type == 0 ? "selected='selected'" : ""; ?>>For sale</option>                        
                        <option value="1" <?php echo $type == 1 ? "selected='selected'" : ""; ?>>Want to buy</option>
                    </select>

                    <br /></label>
                    <p class="clearb">Use , if you want to search for more example "cars, bikes"</p>
                </div>
            </fieldset>
        </div>
        <p><input type="submit" name="search" value="Submit" accesskey="s" /></p>
    </form>
    </div>
</div>


<div class="linkst">
    <div class="inbox">
        <p class="pagelink"><?php echo $paging_links ?></p>
    </div>
</div>

<div id="users1" class="blocktable">
    <h2><span>Found</span></h2>
    <div class="box">
        <div class="inbox">
        <table cellspacing="0">
        <thead>
            <tr>
                <th width="30%" align="left" scope="col">Rubrik</th>
                <th width="20%" align="left" scope="col">Startad</th>
                <th width="20%" align="left" scope="col">Ort</th>
                <th width="20%" align="left" scope="col">Kategori</th>
                <th align="left" scope="col">Pris</th>
            </tr>
        </thead>
        <tbody>
<?php

// Grab the ads
$query = ('SELECT a.id, a.posted, a.header, d.description AS lan, a.ad_type, a.image_url, c.description AS category, a.price FROM '.$db->prefix.'ads AS a, district d, ad_categories c WHERE d.id = a.lan AND c.id = a.category AND a.active = 1 '.$search_string.' ORDER BY a.posted DESC LIMIT '.$start_from.', '.$posts_per_page.'');
//print $query;
$result = $db->query($query) or die (mysql_error());
if ($db->num_rows($result))
{
    while ($ad_data = $db->fetch_assoc($result))
    {
        $ad_title_field = get_title($ad_data);


$today = date("Y-m-d");
$date = date("Y-m-d", $ad_data['posted']);
$s_time = "";
if ($date == $today){
    $s_time = strftime("Idag %H:%M", $ad_data['posted']);
}
else if (strtotime($date) == strtotime("$today -1 day")){
    $s_time = strftime("Igår %H:%M", $ad_data['posted']);
}
else{
    $s_time = strftime("%e %b %H:%M", $ad_data['posted']);
}

?>
                <tr>
                    <td width="30%" align="left" scope="col"><img src="<?php echo $ad_data['image_url'] != "" ? "img/icon_image.gif" : "img/icon_image_none.gif";?>" align="middle" />&nbsp;<a href="ad.php?id=<?php echo $ad_data['id']; ?>"><?php echo "<strong>"; echo $ad_data['ad_type'] == 1 ? "K: " : "S: "; echo "</strong>"; echo $ad_data['header'] ?></a></th>
                    <td width="20%" align="left" scope="col">
<?php echo $s_time; ?></th>
                    <td width="20%" align="left" scope="col"><?php echo $ad_data['lan']  ?></th>
                    <td width="20%" align="left" scope="col"><?php echo $ad_data['category'] ?></th>
                    <td align="left" scope="col"><?php echo $ad_data['price'] ?>:-</th>
                </tr>
<?php

    }
}
else
    echo "\t\t\t".'<tr>'."\n\t\t\t\t\t".'<td class="tcl" colspan="5">No match</td></tr>'."\n";

?>
            </tbody>
            </table>
        </div>
    </div>
</div>

<div class="linksb">
    <div class="inbox">
        <p class="pagelink"><?php echo $paging_links ?></p>
    </div>
</div>
<?php

require PUN_ROOT.'footer.php';
If I for example write ' or '1'='1 I get error:

Code: Select all

You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'or '1'='1' IN BOOLEAN MODE )' at line 3

Re: SQL injection problem

Posted: Thu Jan 20, 2011 2:34 pm
by sockpuppet
echo out the sql as it makes more sense to see it in a whole (well to me anyway).

Re: SQL injection problem

Posted: Thu Jan 20, 2011 2:37 pm
by John Cartwright
Yes, post an example SQL.

And please use [code=php][/code] tags in the future when posting PHP code.

Re: SQL injection problem

Posted: Thu Jan 20, 2011 2:49 pm
by aleborg
Here is the SQL:
CREATE TABLE `ad_categories` (
`id` tinyint(3) unsigned NOT NULL auto_increment,
`description` varchar(255) NOT NULL,
PRIMARY KEY (`id`)
);




CREATE TABLE `district` (
`id` int(11) NOT NULL auto_increment,
`description` varchar(255) NOT NULL default '',
PRIMARY KEY (`id`)
) TYPE=MyISAM AUTO_INCREMENT=1 ;





CREATE TABLE `ads` (
`id` int(10) unsigned NOT NULL auto_increment,
`userid` int(10) unsigned NOT NULL,
`posted` int(10) unsigned NOT NULL,
`header` varchar(255) default NULL,
`body` text NOT NULL,
`image_url` varchar(255) NOT NULL,
`lan` tinyint(4) default NULL,
`name` varchar(255) NOT NULL,
`email` varchar(255) NOT NULL,
`telephone` varchar(50) NOT NULL,
`ad_type` tinyint(1) NOT NULL,
`category` tinyint(4) NOT NULL,
`price` int(11) NOT NULL,
`active` tinyint(1) unsigned NOT NULL default '0',
PRIMARY KEY (`id`),
FULLTEXT KEY `body` (`body`),
FULLTEXT KEY `header` (`header`),
FULLTEXT KEY `header_2` (`header`)
);





ALTER TABLE `ads`
ADD FULLTEXT(body)



ALTER TABLE `ads`
ADD FULLTEXT(header)

Re: SQL injection problem

Posted: Thu Jan 20, 2011 2:52 pm
by John Cartwright
No, we need to see the SQL query that is being executed. Sure we can see your algorithm, but we don't want to spend all day trying to figure out what the problem is when we can identify it with a fully structured SQL query easily.

Basically,

Code: Select all

$query = ('SELECT a.id, a.posted, a.header, d.description AS lan, a.ad_type, a.image_url, c.description AS category, a.price FROM '.$db->prefix.'ads AS a, district d, ad_categories c WHERE d.id = a.lan AND c.id = a.category AND a.active = 1 '.$search_string.' ORDER BY a.posted DESC LIMIT '.$start_from.', '.$posts_per_page.'');
$result = $db->query($query) or die (mysql_error());
to

Code: Select all

$query = ('SELECT a.id, a.posted, a.header, d.description AS lan, a.ad_type, a.image_url, c.description AS category, a.price FROM '.$db->prefix.'ads AS a, district d, ad_categories c WHERE d.id = a.lan AND c.id = a.category AND a.active = 1 '.$search_string.' ORDER BY a.posted DESC LIMIT '.$start_from.', '.$posts_per_page.'');
echo $query;
exit();
P.S., you really need to look up how to use KEYS and how they can highly affect performance if not set correctly. This is for another thread though if you so decide.

Re: SQL injection problem

Posted: Thu Jan 20, 2011 3:26 pm
by aleborg
SELECT * FROM `ads` WHERE 1

Re: SQL injection problem

Posted: Thu Jan 20, 2011 3:47 pm
by John Cartwright
Clearly that's not the right query...

Re: SQL injection problem

Posted: Thu Jan 20, 2011 4:36 pm
by aleborg
John Cartwright wrote:Clearly that's not the right query...
How should it look like then?

Re: SQL injection problem

Posted: Thu Jan 20, 2011 4:42 pm
by John Cartwright
aleborg wrote:
John Cartwright wrote:Clearly that's not the right query...
How should it look like then?
I asked you to post the result of the code I gave you.

You gave me the result of a different query. :?

Re: SQL injection problem

Posted: Thu Jan 20, 2011 5:13 pm
by aleborg
John Cartwright wrote:
aleborg wrote:
John Cartwright wrote:Clearly that's not the right query...
How should it look like then?
I asked you to post the result of the code I gave you.

You gave me the result of a different query. :?
It did not work :/

Re: SQL injection problem

Posted: Fri Jan 21, 2011 8:43 am
by John Cartwright
What do you mean it did not work?

The query I showed you in my post, echo out the ENTIRE query string and post the results of that echo so we all can see.