0

I have a variable that takes the value from previous page, and I want to use that value as my table name. The value passes is an integer so therefore I want my table name as schedule_12 if the value passed is 12. I have used the code below, it does not give any kinds of error but the table was not created in my database. Is there any way to solve this ? Thanks in adance

 <?php
$trNum=$_REQUEST["tr_num"];
 include ("dbConnect.php");


@mysql_query("create table 'schedule_".$trNum."'(sid int primary key 
auto_increment,st_name varchar(20), arr_time varchar(5), dep_time 
varchar(5), halt varchar(5), dist int, day int);");

echo "Schedule created successfully";


?>
6
  • 1
    your suppressing the error using the @ sign, first I would dump that. Commented Jun 30, 2017 at 4:20
  • 1
    Secondly, mysql_* functions are deprecated and will be removed in PHP7 ( I think ), Thirdly be careful of what that name is because this is open to SQL injection, I would do (int)$trNum and cast it to an integer at the very least. But without seeing the source of that variable one cannot be sure... Commented Jun 30, 2017 at 4:21
  • 1
    Fourth table names should not be quoted like a string ... probably the source of the error. Commented Jun 30, 2017 at 4:23
  • always try to use mysqli or PDO method..mysql methods will be invalid after some days... (or) add this line on top of the page error_reporting(1); Commented Jun 30, 2017 at 4:26
  • Hi.. Thanks for the help. I am new to database and working on a project to learn php & MySQL. After removing the @sign from mysql also encountering the same problem..Thanks for your time. Commented Jun 30, 2017 at 4:29

2 Answers 2

1

Something like this

<?php
$trNum=$_REQUEST["tr_num"];
include ("dbConnect.php");


mysql_query("
    CREATE TABLE schedule_".(int)$trNum."
        (
            sid int primary key auto_increment,
            st_name varchar(20), arr_time varchar(5),
            dep_time varchar(5),
            halt varchar(5),
            dist int,
            day int
        )"
);

echo "Schedule created successfully";

Note as I said in the comments, table names should not be quoted like strings. Don't use the @ sign because it will suppress errors, which we need to make sure things work. mysql_* family of functions are deprecated consider using mysqli_* or PDO.

Lastly and very important, because you are concatenating a variable into SQL it leaves you open to SQL injection attacks. In this case there probably isn't much you can do ( like prepared query etc.. ). That variable is not safe because its supplied by the Client, though $_REQUEST. So you need to check it before using it.

I showed you casting but you can check it several ways including Regx

    if( preg_match('/^[0-9]+$/', $trNum ) ){
          ...create table code
    } 

This regx checks that it begins and ends with Numbers and only contains Numbers, which would limit what could be injected into the SQL. There may be better ways to sanitize this, but this should be sufficient in this case.

Casting it is ok, but it may cast a string to 0 and try to create a table named schedule_0 if someone tried to hack it, which is not Ideal. However it's better they get the error message for creating a table that exists then the alternative.

For a quick example of how this could be exploited, a user could supply a value of 0( id int ); UPDATE users SET password = 'password'; -- then your query becomes this.

mysql_query("
    CREATE TABLE schedule_0( id ind ); UPDATE users SET password = 'password'; --
        (
            ... orignal fields ..
        )"
);

So the -- is the start of a comment in SQL, everything past it will not be ran by the DB. Then the 0( id int ); completes your original query creating a minimal table. Lastly we sneak in an Update to change the passwords of all your application users. And then we can login as anyone we want and do all sorts of nasty things.

This is a simplified and imagined case, but it's not to far from what is possible when leaving yourself wide open...

Sign up to request clarification or add additional context in comments.

2 Comments

Thanks for your help, now my problem has been resolved. That (int)$trNum makes my code correct. Thanks for your time..
@AtulAgrawal - please head and understand the implications of SQL injection, it's your responsibility to your users as the developer, to protect their data as best you can.
1

The comments pretty much gave you the answer (as well as some good suggestions). Here is how you'd correct it (using PDO). Notice I removed the quotes in the query, don't forget to sanitise $trNum if it is passed by a user, otherwise SQL injections are possible.

<?php
$trNum=$_REQUEST["tr_num"];

$db = new PDO("dbtype:host=yourhost;dbname=yourdbname;charset=utf8","username","password");
$db->query("create table schedule_" . $trNum . " (sid int primary key 
auto_increment, st_name varchar(20), arr_time varchar(5), dep_time 
varchar(5), halt varchar(5), dist int, day int)");

echo "Schedule created successfully";

?>

2 Comments

still vulnerable to SQL injection, PDO makes no difference in this case. I don't think you can bind for column names or table names with prepared statements. PDO is not a magic bullet that makes everything secure.
@ArtisticPhoenix Read what I wrote again, I'm saying SQL injections are possible if $trNum is not sanitised AKA quotes are not escaped. I'm not saying that PDO helps in anyway against this particular attack.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.