1

I'm currently working on a project and I thought about writing a function to fetch particular data from users table, and so I did, but I'm not sure is this the safest way or not ?

function getuserData($id, $field){

    global $connection;

    $sql = "SELECT `$field` FROM users WHERE id = ?";
    $stmt = mysqli_prepare($connection, $sql);
    mysqli_stmt_bind_param($stmt, "s", $param_id);
    $param_id = $id;
    mysqli_stmt_execute($stmt);
    $result = mysqli_stmt_get_result($stmt);
    while($row = mysqli_fetch_row($result))
    {
        echo $row[0];
    }
}
getuserData($_SESSION['id'], 'name');

Is there a different way to do this, and mainly, is it safe to do it this way ?

w0lfie.
  • 9
  • 2
  • Safe in what way? There are probably infinite ways to do it differently – Dharman Oct 09 '21 at 20:23
  • @Dharman the $sql is my main point of concern, since I'm passing a variable for a data I want to get. Would that be an issue ? – w0lfie. Oct 09 '21 at 20:25
  • Maybe, I can't really say. Generally, you should never ever put variables in SQL query. The most important thing is that you are parameterizing the ID which will protect you against SQL injection, but the `$field` variable is not validated in any way. – Dharman Oct 09 '21 at 20:30
  • Does this answer your question? https://stackoverflow.com/questions/7537377/how-to-include-a-php-variable-inside-a-mysql-statement – Dharman Oct 09 '21 at 20:34
  • What do you mean not validated ? Would filtering through white list make it validated, like it's done in thread you provided ? – w0lfie. Oct 09 '21 at 20:41
  • 1
    Yes. Don't allow just any value. Make sure that the value provided to the function is really one of the allowed column names. – Dharman Oct 09 '21 at 20:56
  • 3
    Although I agree with Dharman, I would also say that although I haven’t seen the rest of your code, this feels very inefficient. Grabbing a whole row versus a single column is usually negligible for performance, and your function feels like it could be called multiple times. I’d fetch the entire row as an array, cache it to a static variable by user ID, and then just return from the array by the field key. This avoids the entire whitelist concept then. – Chris Haas Oct 09 '21 at 21:50

0 Answers0