I received some pen tests results. The results say that anybody can update any record by just changing a certain id.
How could I ensure that the user can only update his own record in this function?
public function actionUpdateProfile()
{
$postdata = file_get_contents("php://input");
$response = array("status" => "1", "message" => "Profile update successful.");
$data = json_decode($postdata);
$model = HosDoctors::model()->findByPk($data->doctor_id);
foreach ($data->fields1 as $field) {
$_POST[$field->name] = $field->value;
}
$enc = NEW bCrypt();
$model->attributes = $_POST;
if ($model->save()) {
$response = array("status" => "1", "message" => "Profil erfolgreich aktualisiert");
} else {
pr($model->getErrors());
}
echo json_encode($response);
die;
}
Would it be sufficient to simply check for
if (cookie == $data->doctor_id)
{
//ok
}
else
{
//we are not logged as the user id that we want to update, so deny updating
die;
}
I am assuming the person has "logged in" in some way so that you know "who they are". While security is complex and is definitely not a one-answer topic; at its simplest level, once you have identified the user, use PHP session handling to persist their identity across one or many http/s requests, then internally access any related information using the session cookie id for the duration of the session.
There are several potential issues in this function (unless you posted a heavily edited version). I'll note them as comments.
public function actionUpdateProfile()
{
$postdata = file_get_contents("php://input");
$response = array("status" => "1", "message" => "Profile update successful.");
// Never initialize responses until you really *must*. Chances that a partially prepared response might be output are slight, but why run risks?
// And actually you **do** reinitialize $response later on!
$data = json_decode($postdata);
// You are not verifying that $data *exists* (i.e. the JSON data was, indeed, JSON). You should check that $data is not NULL and that it does have **all** the required fields and that they are valid.
// This is the point where you validate $data->doctor_id, by the way. Or you check that patient_data matches with whatever you have in your $_SESSION or Session app object.
$model = HosDoctors::model()->findByPk($data->doctor_id);
// This is a bad practice. Yes, you have some code that relies on
// _POST. If necessary, wrap it in another code that will set up
// _POST from an input and then delete it. Otherwise you're leaking
// data into a superglobal. You don't want to do that.
foreach ($data->fields1 as $field) {
$_POST[$field->name] = $field->value;
}
// Why are you initialising $enc?
$enc = NEW bCrypt();
// This is not very good. $_POST could contain *other* information
// unless it's been sanitized outside the function.
// I would prepare a setter function, $model->setArray($data), that
// would verify the validity of the attributes before setting them.
$model->attributes = $_POST;
if ($model->save()) {
$response = array("status" => "1", "message" => "Profil erfolgreich aktualisiert");
} else {
pr($model->getErrors());
}
// This works in 90% of the browsers and scenarios. But I'd
// set up a function that would also send the appropriate
// Content-Type headers to satisfy the remaining 10%.
echo json_encode($response);
die;
// e.g. Utilities::jsonResponse($response);
}
Would it be sufficient to simply check for
if (cookie == $data->doctor_id) { //ok } else { //we are not logged as the user id that we want to update, so deny updating die; }
try checking by user token instead of user-id because user-id can be found within some page url or body but the token is hard to get unless using network sniffing technics.
you can generate user token by any token generation method and then store it as a column in the user table in your database.
token generation method e.g
$token = bin2hex(openssl_random_pseudo_bytes(16));
# or in php7
$token = bin2hex(random_bytes(16));