HiveBrain v1.2.0
Get Started
← Back to all entries
patternphpMinor

Testing filtering of certain characters

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
characterscertaintestingfiltering

Problem

I am a 3rd-year computer science undergraduate. One of my university lecturers has developed his own page for students to submit work. It came up that one student was accused of hacking (sic) by the system. The problem was the characters used in the comment field.

The lecturer claimed that he needed to filter out certain characters in order to stop attacks, and that it was a known problem with PHP. (His system had been broken by students before). I am suspecting poor programming, as he claims that the strings are being executed by PHP.

He told me to try writing a PHP program with a form text field, then input PHP code into the text form. He expects that there to be a vulnerability without filtering the input for bad chars.

prepare("INSERT INTO posts( id, post ) VALUES ( NULL, ? )");
            $statement->bindValue(1, $_POST['data'], SQLITE3_TEXT);
            $statement->execute();
        }

    }
    else {
        $db = new SQLite3("../post.sqlite");

        $db->exec("CREATE TABLE posts( id INTEGER PRIMARY KEY, post TEXT )");
    }
}

function posts(){
    if( file_exists("../post.sqlite") ){
        $db = new SQLite3("../post.sqlite");    

        $query = $db->query("SELECT post from posts ORDER BY id DESC");

        while( $row = $query->fetchArray(SQLITE3_ASSOC)){
            print( "" );
            print( htmlentities( $row['post'] ) );
            print( "" );
        }
    }
}

?>

    Try This
    
        
            
            
            Secret key: 
        
        
        
            
        
    


Without the htmlentities function, I know it would have client side vulnerabilities (running malicious scripts). SQL injection is covered (prepared statements). I don't see how the program can be exploited server side. What (if anything) am I missing?

Solution

It seems fine for me.

Some small changes:

-
I'd change the action to

" method="post">


Using PHP_SELF in the action field of a form

-
I'd consider storing escaped data in the database instead of escaping them on every page requests for performance reasons if the page has lots of visitors.

-
"../post.sqlite" should be a constant.

-
I'd reorganize the code a little bit to use only one SQLite3 instance:

define("DB_FILE", "../post.sqlite");

$dbExists = file_exists(DB_FILE);
$db = new SQLite3(DB_FILE);
if (!$dbExists) {
   $db->exec("CREATE TABLE posts( id INTEGER PRIMARY KEY, post TEXT )");
}

if (isset($_POST['data'])) {
    if( isset($_POST['password']) && $_POST['password'] === "correct horse battery staple" ){
        $statement = $db->prepare("INSERT INTO posts( id, post ) VALUES ( NULL, ? )");
        $statement->bindValue(1, $_POST['data'], SQLITE3_TEXT);
        $statement->execute();
    }
}

function posts($db) {
    $query = $db->query("SELECT post FROM posts ORDER BY id DESC");

    while ($row = $query->fetchArray(SQLITE3_ASSOC)) {
        print( "" );
        print( htmlentities( $row['post'] ) );
        print( "" );
    }
}

?>
[...]

            
[...]

close();
?>


It may be worth creating the table with IF NOT EXISTS in every execution instead of the file_exists check for simplicity: Creating an SQLite table only if it doesn't already exist

Code Snippets

<form action="<?php echo htmlentities($_SERVER['PHP_SELF']); ?>" method="post">
define("DB_FILE", "../post.sqlite");

$dbExists = file_exists(DB_FILE);
$db = new SQLite3(DB_FILE);
if (!$dbExists) {
   $db->exec("CREATE TABLE posts( id INTEGER PRIMARY KEY, post TEXT )");
}

if (isset($_POST['data'])) {
    if( isset($_POST['password']) && $_POST['password'] === "correct horse battery staple" ){
        $statement = $db->prepare("INSERT INTO posts( id, post ) VALUES ( NULL, ? )");
        $statement->bindValue(1, $_POST['data'], SQLITE3_TEXT);
        $statement->execute();
    }
}

function posts($db) {
    $query = $db->query("SELECT post FROM posts ORDER BY id DESC");

    while ($row = $query->fetchArray(SQLITE3_ASSOC)) {
        print( "<pre>" );
        print( htmlentities( $row['post'] ) );
        print( "</pre><hr />" );
    }
}

?>
[...]

            <?php posts($db); ?>
[...]

<?php
    $db->close();
?>

Context

StackExchange Code Review Q#11610, answer score: 6

Revisions (0)

No revisions yet.