Достаточное ли условие для удаления в плане безопасности?

Question

[AlikDex]

Удаление фоточки с альбома пользователя, которую может удалять только пользователь, либо админ.
Итак:
$member['user_id'] - id залогиненого юзера.
$admin - флажок, если юзер с правами админе (упрощено)
$image_id - собственно ид фоточки.

if ( ! $admin ) {
		$user_name = "AND a.user_id = '" . ($member['user_id']) . "'";
	} else {
		$user_name = "";
	}
	
	$sql = "DELETE LOW_PRIORITY i 
			FROM `" . PREFIX . "_images_info` AS i 
			INNER JOIN (
				SELECT album_id
				FROM `" . PREFIX . "_album_info` 
				WHERE album_id={$id} {$user_name} 
			)  AS a USING (album_id)
			WHERE i.album_id=a.album_id AND i.image_id={$image_id}";

Сижу ломаю голову, хватит ли этого от несанкционированного удаления.

П.С.
таблица с фотками напрямую с юзером никак не связаны, только опосредованно через альбом(который в свою очередь связан с владельцем через id).

Answer 1

[Станислав Макаров]

Для безопасности и защиты от несанкционированного удаления вы бы лучше начали использовать prepared_statements. Причем тут они? А притом, что:
1) будут выполняться только те запросы, которые вы сами написали, без риска "дополнения" этих запросов человеком со стороны;
2) логика вашего кода станет более прозрачной, и, следовательно, будет проще выяснить, нет ли нежелательных веток в нем или необработанных ситуаций. Сейчас мне пришлось сначала удивиться, когда я увидел это:
WHERE album_id={$id} {$user_name}
и потом заново все перечитать и понять, что у вас $user_name это оказывается фильтр такой. Вот когда приведете код в порядок, и будете более-менее уверены в отсутствии sql-injection, сами ответите на свой вопрос.

Comments

[AlikDex]

в целом согласен, однако подготовленные выражения в моем случае использовать никак. Для этого необходимо будет переписать полностью класс для работы с бд. Клиенту этого не требуется, а инициативу проявлять я не намерен ;D
В данном случае вставить что-либо сложно, поскольку допустимы только числа, отличные от нуля в большую сторону.
Меня больше интересует такая свзязь через подзапрос. Может лучше было бы сначала селектом проверить наличие фотки в альбоме + возможнные разрешения, а только потом удалять.

[Валерий Рябошапко]

AlikDex: то есть если в $id вдруг окажется "5; drop table PREFIX_images_info", это будет сильно безопасно?

[AlikDex]

с чего оно там окажется?

Answer 2

[Дмитрий Энтелис]

по безопасности imho в целом достаточно.

некоторые замечания:

1. i.album_id=a.album_id я бы переместил в условие join
2. LOW_PRIORITY работает только на myisam - не надо использовать myisam.

Comments

[AlikDex]

1) AS a USING (album_id) уже есть. Спасибо.
2) а что использовать? Иннодб?

[Дмитрий Энтелис]

AlikDex: 1) так это просто использование индекса же. 2) да