0

I'm currently working on a website which has 2 links. Upvote & downvote. Votes are stored in mysql, in a table called "data" with columns "yes" and "no"

I've successfully created a query to update the count of either yes or no, and then echo the value to the page. However, currently the user can spam click the buttons and the count will keep going up.

I've started logging IP addresses with $ip = $_SERVER['REMOTE_ADDR']; & putting them in a table called "ips" with column "ipaddresses".

Now, I want to change my code so that it will query mysql and check the 'ips' table for $ip and if it returns true, then die(); else if... execute upvote query. This will make it so a person can only vote once per IP.

Here is my current code:

<?php
if ($_GET['vote']=="yes") {
    // Connection to database
    $connection=mysqli_connect("hostname-here","username-here","password-here","database-here");
    // Check connection
    if (mysqli_connect_errno())
    {
        echo "Failed to connect to MySQL: " . mysqli_connect_error();
    }
    mysqli_query($connection,"UPDATE data SET yes = (yes + 1) WHERE ID = $_GET[id];");
    mysqli_close($connection);

    echo "Voted.";     
}
?>

Help would be appreciated, I've googled a lot and can't find anything that works. Thanks!

Phil Cooper
  • 3,083
  • 39
  • 63
  • 1
    [Your script is at risk for SQL Injection Attacks.](http://stackoverflow.com/questions/60174/how-can-i-prevent-sql-injection-in-php) – Jay Blanchard Jan 04 '16 at 20:22
  • why not just do it with sessions/tokens? sure beats having to setup another table. and possibly adding a JS button to submit once. Using IP addresses isn't very efficient. – Funk Forty Niner Jan 04 '16 at 20:23
  • I'm not concerned about SQLi. It's localhost and won't be going anywhere. I'm not experienced with sessions/tokens and I just want a simple fix for this. Nothing else. – mark jennings Jan 04 '16 at 20:24
  • 2
    If you're not worried about SQL injection, why are you worried about click-spamming? – Barmar Jan 04 '16 at 20:25
  • ^ exactly. They should be worried. – Funk Forty Niner Jan 04 '16 at 20:25
  • Because I know how I can stop the SQLi if I want to. I don't know how to stop the click spamming. Can we either help, or say nothing? Not looking for 25 irrelevant comments. – mark jennings Jan 04 '16 at 20:25
  • I vote for saying nothing. Who is with me? – Jay Blanchard Jan 04 '16 at 20:27
  • 1
    *Right here Sam* @JayBlanchard oops, we did say something. – Funk Forty Niner Jan 04 '16 at 20:27
  • Then simply don't say anything? I'm trying to get to the point. You don't need to get butthurt just because I said I don't care that it's vulnerable. Trying to solve the task at hand. Simply looking for help with that. If you don't want to help, then don't? you don't need to start a crying argument about it. – mark jennings Jan 04 '16 at 20:28
  • Could you not have a proc that accepts the id and ip, if the ip exists, don't perform the update and error? – Phil Cooper Jan 04 '16 at 21:59

2 Answers2

1

You shouldn't use IP as a deciding factor if someone has already voted. Multiple users can be coming from the same IP.

If you want to do this anyway, you should create a new table. Let's call it "user_action". This table should have a column called IP, and another called VOTE. You will have to log each individual user action, and check the IP before updating your "data" table.

Edit: Some pseudo code to help you more.

Create your table:

CREATE TABLE user_action (IP varchar(39), VOTE tinyint(1));

Simple PHP logic. Just fill in with actual MySQL commands (You apparently already know how to do this from your OP)

    $sSql = "SELECT vote FROM user_action WHERE IP = '" . $_SERVER['REMOTE_ADDR'] ."'";

    If (rowcount > 1) {
        //User already voted, update their answer.
        $sSql = "UPDATE user_action SET vote = " .$_GET['vote']. " WHERE IP = '" . $_SERVER['REMOTE_ADDR'] ."'";
    }
    Else {
        //User hasn't voted, insert their answer
        $sSql = "INSERT INTO user_action (vote, ip) VALUES(" .$_GET['vote']. ", '" . $_SERVER['REMOTE_ADDR'] ."'";
    }

And if you want to tell how many upvotes you have:

$sSql = "SELECT sum(vote) FROM user_action WHERE vote = 1";
Chris Fremgen
  • 4,649
  • 1
  • 26
  • 26
  • This is pretty much saying what I already know. I'm aware that I want to create another table and run a query to check if a row exists, and if it does then die, if it doesn't then continue. I'm just trying to figure out HOW to write that because I've tried a lot of things so far and not working. – mark jennings Jan 04 '16 at 20:31
0

Here you go:

<?php

if ($_GET['vote']=="yes") {
    // Connection to database
    $connection=mysqli_connect("fdb13.your-hosting.net","1789869_gow","niggers1","1789869_gow");
    // Check connection
    if (mysqli_connect_errno())
        {
            die("Failed to connect to MySQL: " . mysqli_connect_error());
        }

    // Check if they've already voted

    $result = mysqli_query($connection, "SELECT COUNT(*) AS already_voted FROM ips WHERE ip = '{$_SERVER['REMOTE_ADDR']}'") or die("Failed to query ips: " . mysqli_error());
    $row = mysqli_fetch_assoc($result);
    if ($row['already_voted']) {
        die("You already voted");
    }

    // Increasing the current value with 1
    mysqli_query($connection,"UPDATE girlsdata SET yes = (yes + 1) WHERE ID = $_GET[id];") or die("Failed to add vote: " . mysqli_error());
    mysqli_close($connection);

    echo "PHP successfully executed. Edit this out later.";    
}
?>
Barmar
  • 741,623
  • 53
  • 500
  • 612
  • Yes, but I want that to execute my upvoting query if it doesn't return an entry. Can you put it into my full code please? – mark jennings Jan 04 '16 at 20:35
  • You don't need `else`, because `die` ends the script. If it gets out of the `if`, the condition must be false. – Barmar Jan 04 '16 at 20:42
  • Doesn't work. Warning: mysqli_query() expects parameter 1 to be mysqli, null given in path-disclosure-here on line 50 Warning: mysqli_fetch_assoc() expects parameter 1 to be mysqli_result, null given in path-disclosure-here on line 51 – mark jennings Jan 04 '16 at 20:44
  • Sorry, forgot the `$connection` argument. – Barmar Jan 04 '16 at 20:54
  • What does `mysqli_error($connection)` show when the query fails? – Barmar Jan 04 '16 at 21:06
  • It just doesn't work. I'll put code on pastebin 1 min – mark jennings Jan 04 '16 at 21:24
  • You're using `$connection` before you do `$connection = mysqli_connect()`. – Barmar Jan 04 '16 at 21:26
  • okay fixed that but it still says "Warning: mysqli_fetch_assoc() expects parameter 1 to be mysqli_result, boolean given in full-path-here on line 52 – mark jennings Jan 04 '16 at 21:30
  • I've updated the answer to show there the check goes in the original code. – Barmar Jan 04 '16 at 21:30
  • You need to check the result of `mysqli_query` to see if there's an error before trying to use it. There are thousands of questions with that error message, they all explain it. – Barmar Jan 04 '16 at 21:31
  • successfully implemented what you wrote but now the problem is that ?id=1 and ?id=2 need to be treated as different pages. – mark jennings Jan 04 '16 at 23:32
  • That's a database design issue. The `ips` table should be keyed on both `ip` and `id`, so you can remember which IDs they already voted on. – Barmar Jan 05 '16 at 00:56