thedigicraft / Atom.CMS

Atom.CMS
56 stars 52 forks source link

Security problem in clean urls? #84

Closed creptor closed 9 years ago

creptor commented 9 years ago

I have used a sql injection test tool called sqlmap, and aparently (I just tried it today) it says there is a url sql injection posibility. And it's probably true due that there is no sanitazer from the slug get to the database connetion.

I would apreciate if you have a solution or if you know how to install iconv to follow this thread

I have search over for this with no current answer. Please help.

creptor commented 9 years ago

buu. I belive I got it, after a day of searching in the documentary, (hate reading) so this is what I have done:

function data_post($dbc, $id) {
    if(is_numeric($id)) {
        $stmt = $dbc->prepare('SELECT * FROM pages WHERE id = ?');//prepared query
    } else {
        $stmt = $dbc->prepare('SELECT * FROM pages WHERE slug = ?');
    }
    $stmt->bind_param('s', $id);//bind the id to the query and stop sql injections
    $stmt->execute();
    $stmt->bind_result($data['id'], $data['title'], $data['header'], $data['label'], $data['body'], $data['user'], $data['slug'], $data['type'], $data['on_field_editable']);
    //bind the $data with the database data (I used the [] params to keep the other variables on the web usefull without changing them)

    while ($stmt->fetch()) {//fetch data from the database
        //regular code for data['body']
        $data['body_nohtml'] = strip_tags($data['body']);
        if($data['body'] == $data['body_nohtml']) {
            $data['body_formatted'] = '<p>'.$data['body'].'</p>';
        } else {
            $data['body_formatted'] = $data['body'];    
        }
        return $data;
    }
}

(don't be idiots, I posted the error at the end of the day, but that doesn't mean that I didn't research before :/)

BloodWolf89 commented 9 years ago

Any website can be hacked! If someone wants in to your site bad enough they will get in!

creptor commented 9 years ago

That doesn't mean that you will make an easy hackeable website. Many peaple could avoid some atacks.(I'm not really inform about this because I find it confusing, but that won't stop me)

creptor commented 9 years ago

The only thing that I read that I undertood was to never trust user imput data, and that includes the urls.

creptor commented 9 years ago

and that is why the author in the videos use mysqli_escape_string, It basically deny any simbols that could be made to get into your database.

BloodWolf89 commented 9 years ago

Ok than why are you complaining about it?

creptor commented 9 years ago

I guess I got anoyed because I had to read a lot sry.

BloodWolf89 commented 9 years ago

No Problem. If the issues are solved, can you close this thread? :)

ghost commented 9 years ago

lol .. the guy just wanted the good

ghost commented 9 years ago

more infos are welcomed any time ;)

creptor commented 9 years ago

the explanation is on the code itself. And I also think I got a better way to do that but, I'm not going back, just read the cuotes in the code

creptor commented 9 years ago

I give here another way of doing this, where you don't need to change anything. I didn't post this first because you need a php version higher than 5.2.

function data_post($dbc, $id) {
if(is_numeric($id)) {
    $stmt = $dbc->prepare('SELECT * FROM pages WHERE id = ?');
} else {
    $stmt = $dbc->prepare('SELECT * FROM pages WHERE slug = ?');
}
$stmt->bind_param('s', $id);
$stmt->execute();
$res = $stmt->get_result();
while ($data = $res->fetch_assoc()) {
    $data['body_nohtml'] = strip_tags($data['body']);
    if($data['body'] == $data['body_nohtml']) {
        $data['body_formatted'] = '<p>'.$data['body'].'</p>';
    } else {
        $data['body_formatted'] = $data['body'];    
    }
    return $data;
}
}

remember that this is inside the data.php or where you place the data_post function