SQL injection problem

Discussions of secure PHP coding. Security in software is important, so don't be afraid to ask. And when answering: be anal. Nitpick. No security vulnerability is too small.

Moderator: General Moderators

Post Reply
aleborg
Forum Newbie
Posts: 5
Joined: Thu Jan 20, 2011 1:37 pm

SQL injection problem

Post 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
sockpuppet
Forum Newbie
Posts: 22
Joined: Tue Jan 18, 2011 8:38 am

Re: SQL injection problem

Post by sockpuppet »

echo out the sql as it makes more sense to see it in a whole (well to me anyway).
User avatar
John Cartwright
Site Admin
Posts: 11470
Joined: Tue Dec 23, 2003 2:10 am
Location: Toronto
Contact:

Re: SQL injection problem

Post by John Cartwright »

Yes, post an example SQL.

And please use [code=php][/code] tags in the future when posting PHP code.
aleborg
Forum Newbie
Posts: 5
Joined: Thu Jan 20, 2011 1:37 pm

Re: SQL injection problem

Post 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)
User avatar
John Cartwright
Site Admin
Posts: 11470
Joined: Tue Dec 23, 2003 2:10 am
Location: Toronto
Contact:

Re: SQL injection problem

Post 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.
aleborg
Forum Newbie
Posts: 5
Joined: Thu Jan 20, 2011 1:37 pm

Re: SQL injection problem

Post by aleborg »

SELECT * FROM `ads` WHERE 1
User avatar
John Cartwright
Site Admin
Posts: 11470
Joined: Tue Dec 23, 2003 2:10 am
Location: Toronto
Contact:

Re: SQL injection problem

Post by John Cartwright »

Clearly that's not the right query...
aleborg
Forum Newbie
Posts: 5
Joined: Thu Jan 20, 2011 1:37 pm

Re: SQL injection problem

Post by aleborg »

John Cartwright wrote:Clearly that's not the right query...
How should it look like then?
User avatar
John Cartwright
Site Admin
Posts: 11470
Joined: Tue Dec 23, 2003 2:10 am
Location: Toronto
Contact:

Re: SQL injection problem

Post 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. :?
aleborg
Forum Newbie
Posts: 5
Joined: Thu Jan 20, 2011 1:37 pm

Re: SQL injection problem

Post 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 :/
User avatar
John Cartwright
Site Admin
Posts: 11470
Joined: Tue Dec 23, 2003 2:10 am
Location: Toronto
Contact:

Re: SQL injection problem

Post 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.
Post Reply