[RESOLVED] Delete row without primary key

[rapio]

Hi all, maybe i posted in wrong forums so ill try here
I have 3 tables
games, membergames, members.

games.games_id is matched to membergames.Games_id and members.user_id is matched to membergames.Member_id.

I want to be able to delete rows from membergames to break the relationship between games and members. I do not have a primary key on the membergames table. So my question is..should I have a primary key there?

The reason im asking is because im having trouble writing my php script to delete the row. So here is an example of how i have another delete working. I need my row to delete similar if possable.

the id is passed through the url like so:
<a href=\"member_commit.php?action=delete&type=user&id={$record->user_id}\">[delete]</a>

The php commit page reads it and deletes the user. But since I dont have a primary key for the membergames page im not sure how to write the code so that it will delete the row. Any help is appreciated.

case "delete": 
      switch ($_GET['type']) { 
        case "user": 
  if (!isset($_GET['do']) || $_GET['do'] != 1) { 
?> 
  <p align="center" style="color:#FF0000"> 
    Are you sure you want to delete this <?php 
    echo $_GET['type']; ?>?<br> 
    <a href="<?php echo $_SERVER['REQUEST_URI']; ?>&do=1">yes</a> 
    or <a href="member_view.php">Index</a> 
  </p> 
<?php 
  } else { 
    if ($_GET['type'] == "user") { 
      // delete reference to member 
    $sql = "DELETE xsm_member, xsm_membergames   

           FROM xsm_member 
           LEFT JOIN xsm_membergames ON xsm_membergames.Member_id = xsm_member.user_id 
           WHERE " . $_GET['type'] . "_id = '" . $_GET['id'] . "'"; 
    }         

  } 

[Piranha]

First: Yes, you should have a primary key. Always. Without exception.

Second: You could use a primary key containing two columns, ie ids from both the other tables.

Third: Your code is vunrable to sql injection. Read about it in the articles part of phpbuilder.com.

Forth: It doesn't matter what column you use to delete, if itis a primary key or not. It is always the same. What I don't understand from your code is why you want to join two tables when you delete. Just make it easy:

// If you want to delete all relationships with table member
$sql = "DELETE FROM membergames WHERE memberid = $variable";
// If you want to delete one particular relationship
$sql = "DELETE FROM membergames WHERE memberid = $variable AND gamesid = $variable2";

[rapio]

Piranha wrote:

First: Yes, you should have a primary key. Always. Without exception.

Second: You could use a primary key containing two columns, ie ids from both the other tables.

Third: Your code is vunrable to sql injection. Read about it in the articles part of phpbuilder.com.

Forth: It doesn't matter what column you use to delete, if itis a primary key or not. It is always the same. What I don't understand from your code is why you want to join two tables when you delete. Just make it easy:

// If you want to delete all relationships with table member
$sql = "DELETE FROM membergames WHERE memberid = $variable";
// If you want to delete one particular relationship
$sql = "DELETE FROM membergames WHERE memberid = $variable AND gamesid = $variable2";

Thanks for the quick response. Im deleting with the join to remove the user from the member table and all reference to the same user from the membergames table.

[rapio]

Piranha
Thanks again for the reply. Im new to coding so i have alot to learn ahead of me. I do have the Pro PHP Security book (purchased last wednesday). Would you mind sharing in my code which part is vulnerable? I have alot of code like this and getting it secure is the next part of my project. I just finished today all my php pages. Security is now what I need to focus on.

Thanks again

[Piranha]

I understand that it is much to learn when you are new and that it takes time and a lot of effort. It is ok not knowing everything and asking a lot in order to learn.

But it seems like you haven't read the 3 articles about sql injection / database injection. If you still have questions after you have read them please post it. But I won't answer if you haven't tried to learn yourself. I belive that you learn best from having to learn, not just getting the answers from others. So please ask specific questions after you have tried to find the answer yourself and you will get answers.

[rapio]

I have Pro PHP Security and have read chapters 11 & 12 at least twice now lol. I understand that some of the steps taken to prevent exploits is making sure the input boxes arent larger than the table field storing the information. Thats simple in itself.
In 12 the book recommends writing a safe function using mysql_real_escape_string.

I edited my code based on the function in the book to look like this:

    if ($_GET['type'] == "user") {
      // delete reference to member
    $sql = "DELETE xsm_member, xsm_membergames  

           FROM xsm_member 
		   LEFT JOIN xsm_membergames ON xsm_membergames.Member_id = xsm_member.user_id 
           WHERE " . mysql_real_escape_string($_GET['type']) . "_id = '" . mysql_real_escape_string($_GET['id']) . "'";
    }        

  }

Is this sufficient to do the job of protecting against injection in my code? Chapter 11 says "The most obviously dangerous characters in any value being used in a database query are quotation marks (whether single or double), because these demarcate string values, and semi-colons, because these demarcate queries. Escaping these three characters stops SQL injection attacks cold."

So would the mysql_real_escape_string at the location i placed them stop this type of attack? Should I have the code somewhere else?

[rapio]

i added this to the code:

           WHERE " . mysql_real_escape_string((int)$_GET['type']) . "_id = '" . mysql_real_escape_string((int)$_GET['id']) . "'"; 

and get this error:
Invalid query: Unknown column '0_id' in 'where clause'

Im going to assume that since

mysql_real_escape_string((int)$_GET['type']) 

is not an integer that is the reason for the error. I then tried this:

           WHERE " . mysql_real_escape_string($_GET['type']) . "_id = '" . mysql_real_escape_string((int)$_GET['id']) . "'"; 

This works without error. So i have added mysql_real_escape_string to remove harmful characters. I also added the (int) because the only thing that should be passed to this delete from the url is the id of the person being deleted. So is this a good start? Did i get way off base with this?

[Piranha]

You seem to have grasped the basics. When it is int values it is enough to convert the value to an int with (int), if it is a string (or anything but numbers) you should use mysql_real_escape_string.

Or even better, if you use PHP5 and MySQL 5 use PDO. I haven't used it myself, so I can't help you out with that, but it handles sql injection without you having to do anything special.