MrAttire Posted August 4, 2023 Share Posted August 4, 2023 Hello Everyone, I am currently working on a project and I want to make sure my forms are secure for users to edit their own listings so I am looking for advice on the best way to insure its secure from any cross scripting or SQL injections Here is my file <?php require_once(__DIR__ .'/../secure/config.php'); if(empty($_POST['place_id'])) { header("Location: $baseurl/user"); } $errors = array(); $transport_smtp = Swift_SmtpTransport::newInstance($smtp_server, $smtp_port, $cfg_smtp_encryption) ->setUsername($smtp_user) ->setPassword($smtp_pass); $mailer = Swift_Mailer::newInstance($transport_smtp); $place_id = $_POST['place_id']; $address = !empty($_POST['address']) ? $_POST['address'] : ''; $area_code = !empty($_POST['area_code']) ? $_POST['area_code'] : 0; $country_code = !empty($_POST['country_code']) ? $_POST['country_code'] : ''; $cat_id = !empty($_POST['category_id']) ? $_POST['category_id'] : ''; $city_id = !empty($_POST['city_id']) ? $_POST['city_id'] : 0; $mon_oc = !empty($_POST['mon_oc']) ? $_POST['mon_oc'] : 0; $mon_oa = !empty($_POST['mon_oa']) ? $_POST['mon_oa'] : 0; $mon_op = !empty($_POST['mon_op']) ? $_POST['mon_op'] : 0; $tue_oc = !empty($_POST['tue_oc']) ? $_POST['tue_oc'] : 0; $tue_oa = !empty($_POST['tue_oa']) ? $_POST['tue_oa'] : 0; $tue_op = !empty($_POST['tue_op']) ? $_POST['tue_op'] : 0; $wed_oc = !empty($_POST['wed_oc']) ? $_POST['wed_oc'] : 0; $wed_oa = !empty($_POST['wed_oa']) ? $_POST['wed_oa'] : 0; $wed_op = !empty($_POST['wed_op']) ? $_POST['wed_op'] : 0; $thur_oc = !empty($_POST['thur_oc']) ? $_POST['thur_oc'] : 0; $thur_oa = !empty($_POST['thur_oa']) ? $_POST['thur_oa'] : 0; $thur_op = !empty($_POST['thur_op']) ? $_POST['thur_op'] : 0; $fri_oc = !empty($_POST['fri_oc']) ? $_POST['fri_oc'] : 0; $fri_oa = !empty($_POST['fri_oa']) ? $_POST['fri_oa'] : 0; $fri_op = !empty($_POST['fri_op']) ? $_POST['fri_op'] : 0; $sat_oc = !empty($_POST['sat_oc']) ? $_POST['sat_oc'] : 0; $sat_oa = !empty($_POST['sat_oa']) ? $_POST['sat_oa'] : 0; $sat_op = !empty($_POST['sat_op']) ? $_POST['sat_op'] : 0; $sun_oc = !empty($_POST['sun_oc']) ? $_POST['sun_oc'] : 0; $sun_oa = !empty($_POST['sun_oa']) ? $_POST['sun_oa'] : 0; $sun_op = !empty($_POST['sun_op']) ? $_POST['sun_op'] : 0; $hol_oc = !empty($_POST['hol_oc']) ? $_POST['hol_oc'] : 0; $hol_oa = !empty($_POST['hol_oa']) ? $_POST['hol_oa'] : 0; $hol_op = !empty($_POST['hol_op']) ? $_POST['hol_op'] : 0; $new_oc = !empty($_POST['new_oc']) ? $_POST['new_oc'] : 0; $new_oa = !empty($_POST['new_oa']) ? $_POST['new_oa'] : 0; $new_op = !empty($_POST['new_op']) ? $_POST['new_op'] : 0; $full_oc = !empty($_POST['full_oc']) ? $_POST['full_oc'] : 0; $contact_email = !empty($_POST['contact_email']) ? $_POST['contact_email'] : ''; $cross_street = !empty($_POST['cross_street']) ? $_POST['cross_street'] : ''; $custom_fields_ids = !empty($_POST['custom_fields_ids']) ? $_POST['custom_fields_ids'] : ''; $delete_existing_pics = !empty($_POST['delete_existing_pics']) ? $_POST['delete_existing_pics'] : array(); $delete_temp_pics = !empty($_POST['delete_temp_pics']) ? $_POST['delete_temp_pics'] : array(); $short_desc = !empty($_POST['short_desc']) ? $_POST['short_desc'] : ''; $description = !empty($_POST['description']) ? $_POST['description'] : ''; $existing_pics = !empty($_POST['existing_pics']) ? $_POST['existing_pics'] : array(); $facebook = !empty($_POST['facebook']) ? $_POST['facebook'] : ''; $pinterest = !empty($_POST['pinterest']) ? $_POST['pinterest'] : ''; $instagram = !empty($_POST['instagram']) ? $_POST['instagram'] : ''; $linkedin = !empty($_POST['linkedin']) ? $_POST['linkedin'] : ''; $tiktok = !empty($_POST['tiktok']) ? $_POST['tiktok'] : ''; $threads = !empty($_POST['threads']) ? $_POST['threads'] : ''; $inside = !empty($_POST['inside']) ? $_POST['inside'] : ''; $latlng = !empty($_POST['latlng']) ? $_POST['latlng'] : ''; $logo = !empty($_POST['uploaded_logo']) ? $_POST['uploaded_logo'] : ''; $neighborhood = !empty($_POST['neighborhood']) ? $_POST['neighborhood'] : ''; $phone = !empty($_POST['phone']) ? $_POST['phone'] : ''; $place_name = !empty($_POST['place_name']) ? $_POST['place_name'] : ''; $zip_code = !empty($_POST['zip_code']) ? $_POST['zip_code'] : ''; $twitter = !empty($_POST['twitter']) ? $_POST['twitter'] : ''; $uploads = !empty($_POST['uploads']) ? $_POST['uploads'] : array(); $videos = !empty($_POST['videos']) ? $_POST['videos'] : array(); $website = !empty($_POST['website']) ? $_POST['website'] : ''; $anonymous = !empty($_POST['anonymous']) ? $_POST['anonymous'] : 0; $wa_area_code = !empty($_POST['wa_area_code']) ? $_POST['wa_area_code'] : ''; $wa_country_code = !empty($_POST['wa_country_code']) ? $_POST['wa_country_code'] : ''; $wa_phone = !empty($_POST['wa_phone']) ? $_POST['wa_phone'] : ''; $cats_arr = !empty($_POST['cats']) ? $_POST['cats'] : array(); $orig_cat_id = !empty($_POST['orig_cat_id']) ? $_POST['orig_cat_id'] : ''; $orig_cat_slug = !empty($_POST['orig_cat_slug']) ? $_POST['orig_cat_slug'] : ''; $wa_area_code = preg_replace("/[^0-9]/", "", $wa_area_code); $wa_country_code = preg_replace("/[^0-9]/", "", $wa_country_code); $wa_phone = preg_replace("/[^0-9]/", "", $wa_phone); $address = is_string($address) ? trim($address) : $address; $area_code = is_string($area_code) ? trim($area_code) : $area_code; $country_code = is_string($country_code) ? trim($country_code) : $country_code; $contact_email = is_string($contact_email) ? trim($contact_email) : $contact_email; $cross_street = is_string($cross_street) ? trim($cross_street) : $cross_street; $description = is_string($description) ? trim($description) : $description; $short_desc = is_string($short_desc) ? trim($short_desc) : $short_desc; $facebook = is_string($facebook) ? trim($facebook) : $facebook; $pinterest = is_string($pinterest) ? trim($pinterest) : $pinterest; $instagram = is_string($instagram) ? trim($instagram) : $instagram; $linkedin = is_string($linkedin) ? trim($linkedin) : $linkedin; $tiktok = is_string($tiktok) ? trim($tiktok) : $tiktok; $threads = is_string($threads) ? trim($threads) : $threads; $inside = is_string($inside) ? trim($inside) : $inside; $latlng = is_string($latlng) ? trim($latlng) : $latlng; $logo = is_string($logo) ? trim($logo) : $logo; $neighborhood = is_string($neighborhood) ? trim($neighborhood) : $neighborhood; $phone = is_string($phone) ? trim($phone) : $phone; $place_name = is_string($place_name) ? trim($place_name) : $place_name; $zip_code = is_string($zip_code) ? trim($zip_code) : $zip_code; $twitter = is_string($twitter) ? trim($twitter) : $twitter; $uploads = is_string($uploads) ? trim($uploads) : $uploads; $website = is_string($website) ? trim($website) : $website; $wa_area_code = is_string($wa_area_code) ? trim($wa_area_code) : $wa_area_code; $wa_country_code = is_string($wa_country_code) ? trim($wa_country_code) : $wa_country_code; $wa_phone = is_string($wa_phone) ? trim($wa_phone) : $wa_phone; $query = "SELECT `userid`, `logo` FROM `places` WHERE `place_id` = :place_id"; $stmt = $conn->prepare($query); $stmt->bindValue(':place_id', $place_id); $stmt->execute(); $row = $stmt->fetch(PDO::FETCH_ASSOC); $place_userid = $row['userid']; $original_logo = !empty($row['logo']) ? $row['logo'] : ''; if($place_userid != $userid) { if(!$is_admin) { die('no permission to edit '.$place_name); } } $query = "SELECT `feat_home` FROM `places` WHERE `place_id` = :place_id"; $stmt = $conn->prepare($query); $stmt->bindValue(':place_id', $place_id); $stmt->execute(); $row = $stmt->fetch(PDO::FETCH_ASSOC); $feat_home = $row['feat_home']; if(!empty($latlng)) { $latlng = str_replace('(', '', $latlng); $latlng = str_replace(')', '', $latlng); $latlng = explode(',', $latlng); $lat = trim($latlng[0]); $lng = trim($latlng[1]); settype($lat, 'float'); settype($lng, 'float'); } else { $lat = $default_lat; $lng = $default_lng; } $twitter = twitter_url(trim($twitter)); $facebook = facebook_url(trim($facebook)); $pinterest = pinterest_url(trim($pinterest)); $instagram = instagram_url(trim($instagram)); $linkedin = instagram_url(trim($linkedin)); $tiktok = instagram_url(trim($tiktok)); $threads = instagram_url(trim($threads)); $website = site_url(trim($website)); $short_desc = mb_substr($short_desc, 0, $short_desc_length); if($city_id > 0) { $query = "SELECT `state_id` FROM `cities` WHERE `city_id` = :city_id"; $stmt = $conn->prepare($query); $stmt->bindValue(':city_id', $city_id); $stmt->execute(); $row = $stmt->fetch(PDO::FETCH_ASSOC); $state_id = $row['state_id']; } else { $state_id = 0; } $status = "pending"; if($cfg_auto_approve_listing) { $status = "approved"; } if($is_admin == 1) { $status = "approved"; } $existing_pics_in_db = array(); $query = "SELECT * FROM `photos` WHERE `place_id` = :place_id"; $stmt = $conn->prepare($query); $stmt->bindValue(':place_id', $place_id); $stmt->execute(); while($row = $stmt->fetch(PDO::FETCH_ASSOC)) { $existing_pics_in_db[] = array('dir' => $row['dir'], 'filename' => $row['filename']); } $custom_fields_ids = explode(',', $custom_fields_ids); $custom_fields = array(); foreach($custom_fields_ids as $v) { $field_key = 'field_'.$v; if(!empty($_POST[$field_key])) { if(!is_array($_POST[$field_key])) { $this_field_value = !empty($_POST[$field_key]) ? $_POST[$field_key] : ''; } else { $this_field_value = !empty($_POST[$field_key]) ? $_POST[$field_key] : array(); } $custom_fields[] = array('field_id' => $v, 'field_value' => $this_field_value); } } $post_token = !empty($_POST['submit_token']) ? $_POST['submit_token'] : 'aaa'; $session_token = isset($_SESSION['submit_token']) ? $_SESSION['submit_token'] : ''; $cookie_token = isset($_COOKIE['submit_token']) ? $_COOKIE['submit_token'] : ''; $result_message = ''; if($post_token == $session_token || $post_token == $cookie_token) { try { $conn->beginTransaction(); if(!empty($logo)) { if(!empty($original_logo)) { if(is_file($pic_basepath.'/logo/'.substr($original_logo, 0, 2).'/'.$original_logo)) { unlink($pic_basepath.'/logo/'.substr($original_logo, 0, 2).'/'.$original_logo); } $folder_path = $pic_basepath.'/logo/'.substr($logo, 0, 2); if (!is_dir($folder_path)) { if(!mkdir($folder_path, 0755, true)) { $has_errors = true; $result_message = 'Error creating logo directory'; } touch($folder_path.'/index.php'); } $path_tmp = $pic_basepath.'/logo-tmp/'.$logo; $path_final = $folder_path.'/'.$logo; if(is_file($path_tmp)) { if(copy($path_tmp, $path_final)) { unlink($path_tmp); } } } } else { $logo = $original_logo; } $query = "UPDATE `places` SET `address` = :address, `area_code` = :area_code, `country_code` = :country_code, `city_id` = :city_id, `mon_oc` = :mon_oc, `mon_oa` = :mon_oa, `mon_op` = :mon_op, `tue_oc` = :tue_oc, `tue_oa` = :tue_oa, `tue_op` = :tue_op, `wed_oc` = :wed_oc, `wed_oa` = :wed_oa, `wed_op` = :wed_op, `thur_oc` = :thur_oc, `thur_oa` = :thur_oa, `thur_op` = :thur_op, `fri_oc` = :fri_oc, `fri_oa` = :fri_oa, `fri_op` = :fri_op, `sat_oc` = :sat_oc, `sat_oa` = :sat_oa, `sat_op` = :sat_op, `sun_oc` = :sun_oc, `sun_oa` = :sun_oa, `sun_op` = :sun_op, `hol_oc` = :hol_oc, `hol_oa` = :hol_oa, `hol_op` = :hol_op, `new_oc` = :new_oc, `new_oa` = :new_oa, `new_op` = :new_op, `full_oc` = :full_oc, `cross_street` = :cross_street, `contact_email` = :contact_email, `short_desc` = :short_desc, `description` = :description, `facebook` = :facebook, `pinterest` = :pinterest, `instagram` = :instagram, `inside` = :inside, `lat` = :lat, `lng` = :lng, `logo` = :logo, `neighborhood` = :neighborhood, `phone` = :phone, `place_name` = :place_name, `zip_code` = :zip_code, `state_id` = :state_id, `status` = :status, `anonymous` = :anonymous, `twitter` = :twitter, `wa_area_code` = :wa_area_code, `wa_country_code` = :wa_country_code, `wa_phone` = :wa_phone, `website` = :website, `linkedin` = :linkedin, `tiktok` = :tiktok, `threads` = :threads, `submission_date` = :submission_date WHERE `place_id` = :place_id"; $stmt = $conn->prepare($query); $stmt->bindValue(':address', $address); $stmt->bindValue(':area_code', $area_code); $stmt->bindValue(':country_code', $country_code); $stmt->bindValue(':city_id', $city_id); $stmt->bindValue(':mon_oc', $mon_oc); $stmt->bindValue(':mon_oa', $mon_oa); $stmt->bindValue(':mon_op', $mon_op); $stmt->bindValue(':tue_oc', $tue_oc); $stmt->bindValue(':tue_oa', $tue_oa); $stmt->bindValue(':tue_op', $tue_op); $stmt->bindValue(':wed_oc', $wed_oc); $stmt->bindValue(':wed_oa', $wed_oa); $stmt->bindValue(':wed_op', $wed_op); $stmt->bindValue(':thur_oc', $thur_oc); $stmt->bindValue(':thur_oa', $thur_oa); $stmt->bindValue(':thur_op', $thur_op); $stmt->bindValue(':fri_oc', $fri_oc); $stmt->bindValue(':fri_oa', $fri_oa); $stmt->bindValue(':fri_op', $fri_op); $stmt->bindValue(':sat_oc', $sat_oc); $stmt->bindValue(':sat_oa', $sat_oa); $stmt->bindValue(':sat_op', $sat_op); $stmt->bindValue(':sun_oc', $sun_oc); $stmt->bindValue(':sun_oa', $sun_oa); $stmt->bindValue(':sun_op', $sun_op); $stmt->bindValue(':hol_oc', $hol_oc); $stmt->bindValue(':hol_oa', $hol_oa); $stmt->bindValue(':hol_op', $hol_op); $stmt->bindValue(':new_oc', $new_oc); $stmt->bindValue(':new_oa', $new_oa); $stmt->bindValue(':new_op', $new_op); $stmt->bindValue(':full_oc', $full_oc); $stmt->bindValue(':contact_email', $contact_email); $stmt->bindValue(':cross_street', $cross_street); $stmt->bindValue(':short_desc', $short_desc); $stmt->bindValue(':description', $description); $stmt->bindValue(':facebook', $facebook); $stmt->bindValue(':pinterest', $pinterest); $stmt->bindValue(':instagram', $instagram); $stmt->bindValue(':inside', $inside); $stmt->bindValue(':lat', $lat); $stmt->bindValue(':lng', $lng); $stmt->bindValue(':logo', $logo); $stmt->bindValue(':neighborhood', $neighborhood); $stmt->bindValue(':phone', $phone); $stmt->bindValue(':place_id', $place_id); $stmt->bindValue(':place_name', $place_name); $stmt->bindValue(':zip_code', $zip_code); $stmt->bindValue(':state_id', $state_id); $stmt->bindValue(':status', $status); $stmt->bindValue(':anonymous', $anonymous); $stmt->bindValue(':twitter', $twitter); $stmt->bindValue(':wa_area_code', $wa_area_code); $stmt->bindValue(':wa_country_code', $wa_country_code); $stmt->bindValue(':wa_phone', $wa_phone); $stmt->bindValue(':website', $website); $stmt->bindValue(':linkedin', $linkedin); $stmt->bindValue(':tiktok', $tiktok); $stmt->bindValue(':threads', $threads); $stmt->bindValue(':submission_date', date('Y-m-d\TH:i:sP', time())); $stmt->execute(); $query = "DELETE FROM `rel_place_cat` WHERE `place_id` = :place_id"; $stmt = $conn->prepare($query); $stmt->bindValue(':place_id', $place_id); $stmt->execute(); foreach($cats_arr as $k => $v) { if($v != $cat_id) { $cats_arr[$k] = array($v, 0); } else { unset($cats_arr[$k]); } } $cats_arr[] = array($cat_id, 1); if(!empty($cats_arr)) { $query = "INSERT IGNORE INTO rel_place_cat(place_id, cat_id, city_id, is_main) VALUES"; $i = 1; foreach($cats_arr as $v) { if(is_numeric($v[0])) { if($i > 1) { $query .= ", "; } $query .= "(:place_id_$i, :cat_id_$i, :city_id_$i, :is_main_$i)"; $i++; } } $stmt = $conn->prepare($query); $i = 1; foreach($cats_arr as $v) { if(is_numeric($v[0])) { $stmt->bindValue(":place_id_$i", $place_id); $stmt->bindValue(":cat_id_$i", $v[0]); $stmt->bindValue(":city_id_$i", $city_id); $stmt->bindValue(":is_main_$i", $v[1]); $i++; } } $stmt->execute(); } if(!empty($delete_temp_pics)) { foreach($delete_temp_pics as $v) { $temp_pic_path = $pic_basepath.'/'.$place_tmp_folder.'/'.$v; if(is_file($temp_pic_path)) { unlink($temp_pic_path); } } } if(!empty($delete_existing_pics)) { $where_clause = ''; foreach($delete_existing_pics as $k => $v) { if(in_array($v, array_column($existing_pics_in_db, 'filename'))) { $key = array_search($v, array_column($existing_pics_in_db, 'filename')); $dir = $existing_pics_in_db[$key]['dir']; $pic_full = $pic_basepath.'/'.$place_full_folder.'/'.$dir.'/'.$v; $pic_thumb = $pic_basepath.'/'.$place_thumb_folder.'/'.$dir.'/'.$v; if(is_file($pic_full)) { unlink($pic_full); } if(is_file($pic_thumb)) { unlink($pic_thumb); } $query = "DELETE FROM `photos` WHERE `filename` = :filename"; $stmt = $conn->prepare($query); $stmt->bindValue(':filename', $v); $stmt->execute(); } } } $query = "SELECT COUNT(*) AS num_pics FROM `photos` WHERE `place_id` = :place_id"; $stmt = $conn->prepare($query); $stmt->bindValue(':place_id', $place_id); $stmt->execute(); $row = $stmt->fetch(PDO::FETCH_ASSOC); $num_pics_in_db = $row['num_pics']; if(!empty($uploads)) { $query = "SELECT photo_id FROM `photos` ORDER BY photo_id DESC LIMIT 1"; $stmt = $conn->prepare($query); $stmt->execute(); $row = $stmt->fetch(PDO::FETCH_ASSOC); $last_photo_id = $row['photo_id']; $dir_num = floor($last_photo_id / 1000) + 1; $dir_full = $pic_basepath.'/'.$place_full_folder.'/'.$dir_num; $dir_thumb = $pic_basepath.'/'.$place_thumb_folder.'/'.$dir_num; if (!is_dir($dir_full)) { mkdir($dir_full, 0777, true); } if (!is_dir($dir_thumb)) { mkdir($dir_thumb, 0777, true); } $tmp_folder = $pic_basepath.'/'.$place_tmp_folder; $pic_count = 1; foreach($uploads as $k => $v) { $tmp_file = $tmp_folder.'/'.$v; if($pic_count + $num_pics_in_db < $max_pics + 1) { if(copy($tmp_file, $dir_full.'/'.$v)) { $stmt = $conn->prepare('INSERT INTO photos(place_id, dir, filename) VALUES(:place_id, :dir, :filename)'); $stmt->bindValue(':place_id', $place_id); $stmt->bindValue(':dir', $dir_num); $stmt->bindValue(':filename', $v); $stmt->execute(); } smart_resize_image($tmp_file, null, $global_thumb_width, $global_thumb_height, false, $dir_thumb.'/'.$v, true, false, 85); $query = "DELETE FROM `tmp_photos` WHERE `filename` = :filename"; $stmt = $conn->prepare($query); $stmt->bindValue(':filename', $v); $stmt->execute(); $pic_count++; } else { if(is_file($tmp_file)) { unlink($tmp_file); } } } } $query = "DELETE FROM `rel_place_custom_fields` WHERE `place_id` = :place_id"; $stmt = $conn->prepare($query); $stmt->bindValue(':place_id', $place_id); $stmt->execute(); $custom_fields = array_unique($custom_fields, SORT_REGULAR); foreach($custom_fields as $v) { if(!is_array($v['field_value'])) { if(!empty($v['field_value'])) { $query = "INSERT INTO rel_place_custom_fields(place_id, field_id, field_value) VALUES(:place_id, :field_id, :field_value)"; $stmt = $conn->prepare($query); $stmt->bindValue(':place_id', $place_id); $stmt->bindValue(':field_id', $v['field_id']); $stmt->bindValue(':field_value', $v['field_value']); $stmt->execute(); } } else { foreach($v['field_value'] as $v2) { if(!empty($v2)) { $query = "INSERT INTO rel_place_custom_fields(place_id, field_id, field_value) VALUES(:place_id, :field_id, :field_value)"; $stmt = $conn->prepare($query); $stmt->bindValue(':place_id', $place_id); $stmt->bindValue(':field_id', $v['field_id']); $stmt->bindValue(':field_value', $v2); $stmt->execute(); } } } } $query = "DELETE FROM `videos` WHERE `place_id` = :place_id"; $stmt = $conn->prepare($query); $stmt->bindValue(':place_id', $place_id); $stmt->execute(); if(!empty($videos)) { foreach($videos as $v) { $query = "INSERT INTO videos (place_id, video_url) VALUES(:place_id, :video_url)"; $stmt = $conn->prepare($query); $stmt->bindValue(':place_id', $place_id); $stmt->bindValue(':video_url', $v); $stmt->execute(); } } $conn->commit(); $result_message = $txt_success; if($cfg_enable_sitemaps) { if($orig_cat_id != $cat_id) { $permalink_arr = explode('/', $cfg_permalink_struct); if(in_array('%category%', $permalink_arr)) { $orig_listing_link = get_listing_link($place_id, '', $orig_cat_id, $orig_cat_slug, $city_id, '', '', $cfg_permalink_struct); sitemap_remove_url($orig_listing_link); $new_listing_link = get_listing_link($place_id, '', $cat_id, '', $city_id, '', '', $cfg_permalink_struct); sitemap_add_url($new_listing_link); } } else { $listing_link = get_listing_link($place_id, '', $cat_id, '', $city_id, '', '', $cfg_permalink_struct); sitemap_update_lastmod($listing_link); } } } catch(PDOException $e) { $conn->rollBack(); $result_message = $e->getMessage(); } unset($_SESSION['submit_token']); } $result_message = str_replace('%place_name%', $place_name, $result_message); $txt_main_title = str_replace('%place_name%', $place_name, $txt_main_title); $canonical = $baseurl.'/user/process-edit-listing'; Any help would be appreciated ... Quote Link to comment Share on other sites More sharing options...
MrAttire Posted August 4, 2023 Author Share Posted August 4, 2023 I guess no one sees any issues? Quote Link to comment Share on other sites More sharing options...
mac_gyver Posted August 4, 2023 Share Posted August 4, 2023 (edited) the code for any page should be laid out in this general order - initialization post method form processing get method business logic - get/produce data needed to display the page html document you should keep the form data as a set in a php array variable, then operate on elements in this array variable throughout the rest of the code. this will eliminate the need to create discrete variables for every field and will let you do things like trim all the data at once, with one single line of code, dynamically validate the data, using a data driven design, and dynamically process the data, so that you don't need to write out repetitive code for every field. you should first trim, mainly so that you can detect if all white-space characters were entered, then validate all input data before using it, storing user/validation errors in an array using the field name as the main array index. to enforce user permissions, you would test at the earliest point in whatever operation you are performing if there is a logged in user and if that user has permission to perform the current operation and is the owner or an administrator of the data being being operated on. in your current code, the permission test for ownership of the place data is somewhere near the middle of the code. this should be right after the point where you have determined that there is form data and that the submit_token is valid. the post method form processing code should - detect if a post method form was submitted before referencing any of the form data. detect if the total size of the form data exceeded the post_max_size setting. if this occurs, both the $_POST and $_FILES data will be empty and there's no point in trying to use any of the form data because it won't exist. trim all the form data at once. verify the submit_token. determine ownership of the data being edited. validate all the input data. after the end of the validation logic, if there are no errors (the array holding the user/validation errors is empty), use (process) the form data. if the processing of the form data could produce duplicate errors for data that must be unique, you would detect this and add errors to the array holding the user/validation errors letting the user know what was wrong with the data that they submitted. after processing the form data, if there are no errors, redirect to the exact same url of the current page to cause a get request for that url. this will prevent the browser from trying to resubmit the form data if that page is browsed away from and back to or reloaded. every redirect needs an exit/die statement to stop php code execution. if you want to display a one-time success message, store it in a session variable, then test, display, and clear that session variable at the appropriate location in the html document. if there are errors at item #7 or #9 on this list, the code will continue on to display the html document, test for and display any user/validation errors, redisplay the form, populating field values with the submtited form data. any value you output in a html context should have htmlentities() applied to it to help prevent cross site scripting. to allow you to initially query for the existing data, then use the submitted form data to populate the form fields, you would only query to get the existing data if the form has never been submitted, then fetch the data into the same php array variable that has already been mentioned for holding the form data. Edited August 4, 2023 by mac_gyver Quote Link to comment Share on other sites More sharing options...
mac_gyver Posted August 4, 2023 Share Posted August 4, 2023 addition points about the posted code - keeping the form data as a set in a php array variable and dynamically validating/processing the data will eliminate most of that code. i see you have an array to hold errors, but are not using it. by definition all post/get/cookie data are strings, regardless of the value they hold, and using is_string() on them will always be true. if you use simple ? positional prepared query place-holders and use implicit binding, by supplying an array to the ->execute([...]) call, all the database specific code will be simplified. if you set the default fetch mode to assoc when you make the PDO connection, you don't need to specify it in each fetch statement. don't copy variables to other variables for nothing. just appropriately name and use the original variables that data is in. don't run multiple queries to get different pieces of data from the same table and row. list out the columns you are SELECTing in a query. you can use the PDO fetchAll() method instead of looping to build an array of the fetched data. when you loop to build the multi-row insert query VALUE terms, you can build the array of inputs that you will supply to the ->execute([...]) call in the same loop. also, if you build the terms in an array, then implode the array with the ',' character, you can eliminate the $i variable and the conditional logic. for the SELECT COUNT(*) ... query, you can just use the fetchColumn() method to get the count value. the INSERT INTO photos... query should be prepared once, before the start of the looping, then just call the execute([...]) method with each set of values as an array to the execute call inside the loop. you would only catch database exceptions for user recoverable errors, such as when inserting/updating duplicate or out of range submitted values. for all other database errors, just let php catch and handle the exceptions, where php will use its error related settings to control what happens with the actual error information (database statement errors will 'automatically' get displayed/logged the same as php errors.) Quote Link to comment Share on other sites More sharing options...
MrAttire Posted August 4, 2023 Author Share Posted August 4, 2023 Wow, thank you for your input! That was definitely detailed and I appreciate the time it took... Can you provide me an example of how you would have wrote that file more securely ? Also are you for hire? Quote Link to comment Share on other sites More sharing options...
mac_gyver Posted August 5, 2023 Share Posted August 5, 2023 do you have any specific questions about any of the points that were made? if you are just starting out, i recommend that you start with one form field of one type, e.g. text since it is the most common type, and get your code fully working and tested with that single form field. then, pick one field of a different type, and repeat for each type of field. then you can worry about all the code needed for the rest of the fields. Quote Link to comment Share on other sites More sharing options...
Strider64 Posted August 5, 2023 Share Posted August 5, 2023 (edited) I just wanted to add that when I took a course in web design one of the things to do is keep the form simple as possible. I'm assuming all that data is coming from a form? If that case I would simplify it down a bit as most visitors will leave that website if they see a HTML form that l large. An example of what I'm talking about. <form class="registerStyle" action="register.php" method="post"> <input id="secret" type="hidden" name="user[secret]" value=""> <div id="qr-code-container" class="d-none"> <img id="qr-code-image" src="" alt="QR Code"> <p id="qr-code-info">Scan QR Code for 2FA</p> </div> <div class="first"> <label for="first_name">First Name</label> <input id="first_name" type="text" name="user[first_name]" value="" tabindex="2" required> </div> <div class="last"> <label for="last_name">Last Name</label> <input id="last_name" type="text" name="user[last_name]" value="" tabindex="3" required> </div> <div class="screenName"> <label class="text_username" for="username">Username <span class="error"> is unavailable!</span> </label> <input id="username" class="io_username" type="text" name="user[username]" value="" tabindex="4" required> </div> <div class="telephone"> <label for="phone">Phone (Optional)</label> <input id="phone" type="tel" name="user[phone]" value="" placeholder="555-555-5555" pattern="[0-9]{3}-[0-9]{3}-[0-9]{4}" tabindex="5"> </div> <div class="emailStyle"> <label class="emailLabel" for="email">Email <span class="emailError"> is invalid</span> </label> <input id="email" type="email" name="user[email]" value="" tabindex="1" autofocus required> </div> <div class="password1"> <label for="passwordBox">Password</label> <input class="passwordBox1" id="passwordBox" type="password" name="user[password]" value="" tabindex="6" required> <label for="passwordVisibility">Show Passwords (private computer)</label> <input class="passwordBtn1" id="passwordVisibility" type="checkbox" tabindex="7"> </div> <div class="password2"> <label for="redoPassword">ReEnter Password</label> <input class="passwordBox2" id="redoPassword" type="password" name="user[comfirm_password]" value="" tabindex="8" required> </div> <div class="birthday"> <label for="birthday">Birthday (optional)</label> <input id="birthday" type="date" name="user[birthday]" value="1970-01-01" tabindex="9"> </div> <div class="submitForm"> <button class="submitBtn" id="submitForm" type="submit" name="submit" value="enter" tabindex="10"> Submit </button> </div> </form> or at least pull the data in easier -> Example: $user = $_POST['user']; Edited August 5, 2023 by Strider64 Quote Link to comment Share on other sites More sharing options...
MrAttire Posted August 6, 2023 Author Share Posted August 6, 2023 (edited) Thank you strider, and mac_gyver It seems they keep spoofing accounts to gain access, my account for example because userid = 1 it is_admin so therefore they can change ownership of the links, edit my profile etc... and even approve listings in the admin panel we use a whitelist so if there IP is not present they can not access it... But anything user side is fair game... We are fairly new to PHP PDO and we bought this script from an author who says it's not the script ... which obviously is not the case....as it keeps happening So with your help where do start? Eager to learn and even pay for you time if you like as well there are 8 or 9 files that users access to make changes, login and edit ... I also see the script is supposed to verify the user has that privilege as I see this in the create listing $submit_token = uniqid('', true); $_SESSION['submit_token'] = $submit_token; $_COOKIE['submit_token'] = $submit_token; I await your reply Edited August 6, 2023 by MrAttire Quote Link to comment Share on other sites More sharing options...
MrAttire Posted August 6, 2023 Author Share Posted August 6, 2023 Oops I meant to post this in the user/sign-in l see it verifies the user but I don't know if it's called in the other files like edit etc... You can see it verifies user, cookie login token etc, <?php require_once(__DIR__ .'/../inc/config.php'); $referrer = (isset($_SERVER['HTTP_REFERER'])) ? $_SERVER['HTTP_REFERER'] : $baseurl; $allow_referrer = array('advanced-results', 'advanced-search', 'categories', 'claim', 'upgrade', 'provinces', 'states', 'contact', 'coupon', 'coupons', 'home', 'listing', 'listings', 'msg', 'post', 'posts', 'post', 'posts', 'profile', 'search',); foreach($allow_referrer as $v) { $pos = strpos($referrer, $v); if($pos == false) { $referrer = $baseurl; } } $wrong_pass = 0; $email_already_used = 0; $user_registered = true; $account_pending = 0; $show_form = true; if($_SERVER['REQUEST_METHOD'] == 'POST') { $email = !empty($_POST['email']) ? $_POST['email'] : ''; $password = !empty($_POST['password']) ? $_POST['password'] : ''; $email = trim($email); $password = trim($password); $stmt = $conn->prepare("SELECT `id`, `password`, `status` FROM `users` WHERE `email` = :email"); $stmt->bindValue(':email', $email); $stmt->execute(); $row = $stmt->fetch(PDO::FETCH_ASSOC); $userid = !empty($row['id']) ? $row['id'] : ''; $password_hashed = !empty($row['password']) ? $row['password'] : ''; $status = !empty($row['status']) ? $row['status'] : ''; if(empty($row)) { $user_registered = false; } if($status == 'approved') { if(password_verify($password, $password_hashed)) { $token = bin2hex(openssl_random_pseudo_bytes(16)); $cookie_val = "$userid-localhost-$token"; setcookie('loggedin', $cookie_val, time()+86400*30, $install_path, '', '', true); record_tokens($userid, 'localhost', $token); $_SESSION['user_connected'] = true; $_SESSION['userid'] = $userid; $_SESSION['loggedin_token'] = $token; header("Location: $referrer"); } else { $wrong_pass = 1; } } if($status == 'pending') { $show_form = false; $account_pending = true; } } if(isset($_GET['provider'])) { $email_already_used = 0; $provider_name = $_GET['provider']; $referrer = (isset($_GET['referrer'])) ? $_GET['referrer'] : ''; $config = __DIR__.'/../inc/hybridauth-config.php'; try { $hybridauth = new Hybrid_Auth($config); $adapter = $hybridauth->authenticate($provider_name); $user_profile = $adapter->getUserProfile(); } catch( Exception $e ) { $exception = $e->getMessage(); die($exception); } $provider_user_id = $user_profile->identifier; $first_name = !empty($user_profile->firstName) ? $user_profile->firstName : ''; $last_name = !empty($user_profile->lastName) ? $user_profile->lastName : ''; $email = isset($user_profile->email) ? $user_profile->email : $provider_user_id; $count = 0; $stmt = $conn->prepare("SELECT COUNT(*) AS total_rows FROM `users` WHERE `email` = :email AND `hybridauth_provider_name` = 'local'"); $stmt->bindValue(':email', $email); $stmt->execute(); $count = $stmt->fetchColumn(); if($count > 0) { $email_already_used = 1; } if($email_already_used == 0) { $stmt = $conn->prepare("SELECT COUNT(*) FROM `users` WHERE `hybridauth_provider_name` = :provider_name AND `hybridauth_provider_uid` = :provider_user_id"); $stmt->bindValue(':provider_name', $provider_name); $stmt->bindValue(':provider_user_id', $provider_user_id); $stmt->execute(); $count = $stmt->fetchColumn(); if($count < 1) { $stmt = $conn->prepare("SELECT COUNT(*) FROM `users` WHERE `email` = :email"); $stmt->bindValue(':email', $email); $stmt->execute(); $count2 = $stmt->fetchColumn(); if($count2 < 1) { $password = generatePassword(); $password_hashed = password_hash($password, PASSWORD_BCRYPT); $stmt = $conn->prepare('INSERT INTO users(email, password, first_name, last_name, hybridauth_provider_name, hybridauth_provider_uid, created, status) VALUES(:email, :password, :first_name, :last_name, :hybridauth_provider_name, :hybridauth_provider_uid, NOW(), :status)'); $stmt->bindValue(':email', $email); $stmt->bindValue(':password', $password_hashed); $stmt->bindValue(':first_name', $first_name); $stmt->bindValue(':last_name', $last_name); $stmt->bindValue(':hybridauth_provider_name', $provider_name); $stmt->bindValue(':hybridauth_provider_uid', $provider_user_id); $stmt->bindValue(':status', 'approved'); $stmt->execute(); $stmt = $conn->prepare("SELECT `id` FROM `users` WHERE `email` = :email AND `password` = :password"); $stmt->bindValue(':email', $email); $stmt->bindValue(':password', $password_hashed); $stmt->execute(); $row = $stmt->fetch(PDO::FETCH_ASSOC); $userid = $row['id']; $photo_url = (isset($user_profile->photoURL)) ? $user_profile->photoURL : ''; } else { $email_already_used = 1; } } else { $stmt = $conn->prepare("SELECT `id` FROM `users` WHERE `hybridauth_provider_name` = :provider_name AND `hybridauth_provider_uid` = :provider_user_id"); $stmt->bindValue(':provider_name', $provider_name); $stmt->bindValue(':provider_user_id', $provider_user_id); $stmt->execute(); $row = $stmt->fetch(PDO::FETCH_ASSOC); $userid = $row['id']; } if(!empty($userid)) { $token = bin2hex(openssl_random_pseudo_bytes(16)); record_tokens($userid, $provider_name, $token); $_SESSION['user_connected'] = true; $_SESSION['userid'] = $userid; $_SESSION['loggedin_token'] = $token; $cookie_val = "$userid-$provider_name-$token"; setcookie('loggedin', $cookie_val, time()+86400*30, $install_path, '', '', true); header("Location: $baseurl"); } } } $canonical = $baseurl.'/user/sign-in'; Quote Link to comment Share on other sites More sharing options...
mac_gyver Posted August 6, 2023 Share Posted August 6, 2023 given that the redirects you have shown don't halt php code execution, by having an exit/die statement, if pages that should require a logged in user to access them can be accessed anyways, it is likely that the login check code on the 'secured' web pages doesn't halt php code execution if there is no logged in user. if there's no exit/die statement, all the rest of the code on the page still executes while the browser is performing the redirect. the first posted code, for the places edit processing, doesn't show any login check code at all (though it could be in the config.php file.) if this code 'assumes' that the only way to reach the protected pages is because this code redirected the user to that page, this is incorrect. anyone or anything can request any page. each 'secure' page must test for and prevent access by non-logged in users. Quote Link to comment Share on other sites More sharing options...
mac_gyver Posted August 6, 2023 Share Posted August 6, 2023 here's a point i saw while looking at the logic in this code that shows just how badly it is written - by first using empty() to test the input data, then trimming it later, values that consist of all white-space characters will pass the empty() test, but become an empty string after the trim() call, causing this code to use empty data values. Quote Link to comment Share on other sites More sharing options...
MrAttire Posted August 6, 2023 Author Share Posted August 6, 2023 (edited) SO for an example how would you write it ... example is this secure $stmt = $conn->prepare('INSERT INTO users(first_name, last_name, email, b_year, b_month, b_day, city_name, state_id, country_id, password, created, hybridauth_provider_name, ip_addr, status, subscriber) VALUES(:first_name, :last_name, :email, :b_year, :b_month, :b_day, :city_name, :state_id, :country_id, :password, NOW(), :hybridauth_provider_name, :ip, :status, :subscriber)'); $stmt->bindValue(':first_name', $fname); $stmt->bindValue(':last_name', $lname); $stmt->bindValue(':email', $email); $stmt->bindValue(':b_year', $b_year); $stmt->bindValue(':b_month', $b_month); $stmt->bindValue(':b_day', $b_day); $stmt->bindValue(':city_name', $city_name); $stmt->bindValue(':state_id', $state_id); $stmt->bindValue(':country_id', $country_id); $stmt->bindValue(':password', $password_hashed); $stmt->bindValue(':hybridauth_provider_name', 'local'); $stmt->bindValue(':ip', $ip); $stmt->bindValue(':subscriber', $subscriber); $stmt->bindValue(':status', 'pending'); if($stmt->execute()) { $user_created = 1; or is there a better way? Edited August 6, 2023 by MrAttire Quote Link to comment Share on other sites More sharing options...
mac_gyver Posted August 7, 2023 Share Posted August 7, 2023 20 hours ago, MrAttire said: is this secure yes, but a prepared query's primary purpose is only to prevent sql special characters in a value from being able to break the sql query syntax, which is how sql injection is accomplished, provided you are either using a true/actual prepared query (requires a specific configuration option to be set when the database connection is made) or if using an emulated prepared query (which should be avoided whenever possible), you have set the character set when you make the database connection to match your database table's character set (this should be set in any case, so that a character conversion won't occur over the connection, but is rarely set at all.) if the code leading up to the query doesn't correctly enforce user permissions, detect if a form has been submitted, validate data, or halt php code execution when needed, the execution of the query will occur with whatever data was supplied to it by whoever or whatever submitted the data to the web page. as to the code/query you have posted above - if you build the sql query statement in a php variable, it makes debugging easier, helps prevent typo errors by separating the sql query syntax as much as possible from the php syntax, and allows you to see common code used for each query that can be consolidate into user functions/classes to eliminate repetitive code. the email column must be defined as a unique index, this query then needs exception error handling to detect if a duplicate index error (number) has occurred, and setup a message for the user that the email address is already in use. there should be a single DATE column for the date of birth, so that you can easily perform comparisons, sorting, formatting, or date arithmetic on the value. for the static/literal values, you can just put them directly into the sql query statement. exceptions should be used for database statement errors. this is the default setting now in php8+. when using exceptions for database statement errors, you would remove any existing conditional error handling logic (it will no longer get executed upon an error and any true conditional logic will always be true.) also, refer to item #8 in my first post in this thread and item #13 in my second post. if the query uses simple ? positional prepared query place-holders and implicit binding, by supplying an array to the executed call, all that verbose copy/pasting and typing can be eliminated. and by dynamically processing the data, using a data-driven design, you can eliminate all the bespoke logic building and executing the query entirely. Quote Link to comment Share on other sites More sharing options...
Recommended Posts
Join the conversation
You can post now and register later. If you have an account, sign in now to post with your account.