Security of a custom table in JsGrid

Question

This code shows a table's records via JavaScript using a JsGrid file and can edit or delete by AJAX.

This is my topic How can I load rows in JsGrid table that got from php file?, The problem I am not sure the data is secure during inserting and loading or not?

Is it really secure?

The names of PHP files are: newsConf, controll and getnewscat

HTML file: basic.html

in controll.php:

public function newsConfig(){ $this->CONN = new Conn();//class from external page to connect DB try{
    $dfnet = $this->CONN->open_connect();
    $qnco = mysqli_query($dfnet,"select * from category");
    if(!$qnco) return "";
    else{
        while($qncoarray = mysqli_fetch_assoc($qnco)){
            //here I try copy rows into array
            $nnopCo[] = array(
                'ID' => $qncoarray['ID'],
                'Name' => $qncoarray['Name']
            );
        }
        return $nnopCo;
    }
    $this->CONN->close_connect($dfnet); } catch(Exception $er){ }

in getnewscat.php:

<?php require_once "../../bin/controll.php";
$db_controll = new Controll();
$cat_news = new ArrayObject($db_controll->newsConfig());

header('Content-type: application/json');
echo json_encode($cat_news->getArrayCopy());

?>

in basic.html: is the same file from jsgrid demo, but I change the code in JavaScript and canceled the db.js file

    <script>
            $(function() {
                {

                $("#jsGrid").jsGrid({
                    height: "70%",
                    width: "50%",//100%
                    selecting: false,
                    filtering: false,
                    editing: false,
                    sorting: false,
                    paging: true,
                    autoload: true,
                    pageSize: 15,
                    pageButtonCount: 5,
                    controller: {
                        loadData: function(filter) {
return $.ajax({url: "../bin/getnewscat.php",data:filter
                            });

                        }
    },
                    fields: [
                        { name: "ID", type: "number", width: 50 },
                        { name: "Name", type: "text", width: 50},
                        { type: "control", modeSwitchButton: false, deleteButton: false }
                    ]
               });
    $(".config-panel input[type=checkbox]").on("click", function() {
                    var $cb = $(this);
                    $("#jsGrid").jsGrid("option", $cb.attr("id"), $cb.is(":checked"));
                });

            });
        </script>

In newsConf.php: that file should call basic.html and give the result by using an iframe:

<iframe name="demo" src="jsgrid-1.2.0/demos/basic.html"></iframe>

Answer 1

As far as I can see you are not using user input in your queries so there really isn't any security issue. When using user input in queries always make sure it's through prepared statements.

Answer 2

Response

xReprisal is correct - it appears that the user input is not used in the database queries; the query appears to simply select all rows from one database table.

Feedback

PHP

JSON encoded array

I haven't used ArrayObject::getArrayCopy() before but see it returns a copy of an array. Why return a copy of the array? Why not just return the array? It isn't like the array returned is modified by the code that calls that method - it merely is used in the response... Also, it only accepts an array or an object, so in the case that the controller method newsConfig() returns an empty string literal (i.e. when the call to mysqli_query() returns a value evaluating to false), an exception is thrown with the following message:

Passed variable is not an array or object

So I would suggest not using the ArrayObject class, always have that controller method return an array and simply pass that array to json_encode(). Something like the (untested) code below in getnewscat.php:

<?php require_once "../../bin/controll.php";
$db_controll = new Controll();

header('Content-type: application/json');
echo json_encode($db_controll->newsConfig());

?>

Unreachable code in controller method

In the controller method newsConfig(), it appears that there are two return statements, and those come before the line that would close the database connection (i.e. $this->CONN->close_connect($dfnet);) - thus that line is unreachable. To ensure that the connection is closed properly, one would have to store the return value in a variable and return it after closing the connection - something like the code below. Also, presuming the answer to the question in the previous section is that there isn't really a need to copy the array, just return an array in all cases, even if it is empty...

warning- untested code:

public function newsConfig(){ 
    $return = array();
    $this->CONN = new Conn();//class from external page to connect DB 
    try{
        $dfnet = $this->CONN->open_connect();
        $qnco = mysqli_query($dfnet,"select * from category");
        //if(!$qnco) return "";            else{
        if ($qnco) { //flipped logic
            while($qncoarray = mysqli_fetch_assoc($qnco)){
                //here I try copy rows into array
                $return[] = array(
                    'ID' => $qncoarray['ID'],
                    'Name' => $qncoarray['Name']
                );
            }
        }
        $this->CONN->close_connect($dfnet); 
    } 
    catch(Exception $er){ }
    return $return;
}

JS

cache DOM references

The JS code provided only shows two occurrences of $("#jsGrid") but it would be good to store that in a variable, which will reduce DOM lookups for that element to one.

var gridElement = $("#jsGrid");

...
gridElement.jsGrid({ ... });

//in the callback:
    gridElement.jsGrid("option", $cb.attr("id"), $cb.is(":checked"));

Also, if Ecmascript-2015 (A.K.A. Es-6) is supported by target browsers, use const instead of var, since that value should not be re-assigned (see browser compatibility).

const gridElement = $("#jsGrid");