Jump to content

mac_gyver

Staff Alumni
  • Content Count

    4,326
  • Joined

  • Days Won

    114

Everything posted by mac_gyver

  1. firstly, you should NOT modify/sanitize user input, except perhaps to trim it. by modifying it, you are changing the meaning of it. (a few versions ago of this forum software, the email addresses were 'sanitized' inconsistently, resulting in multiple values mapping to the same account and a hacker was able to do a password recovery for an admin using the hacker's email address, which allowed the hacker to log in as that admin and all the user's data was copied.) what you should do instead is to just validate the user input, to make sure it is suitable within the context of your application. if the data is not valid, don't use it at all and output a message to the user telling them what was wrong with the submitted data. next, the regex pattern you have in your example isn't what you will use for all possible user input. things like names, text content (like we are typing here in the forum) , or email addresses require other characters like single-quotes or strings that are hexadecimal (0-9 and a-f) values. as soon as you allow these and put them directly into the sql query statement, you are open to sql injection. when you get to the database layer, all you should be concerned with is protecting against sql special characters in the data from breaking the sql query syntax (which is how sql injection is accomplished.) you should not care about where the data came from, what it means, or what type of validation/sanitizing it may or may not have received prior to that point. the simplest , most general-purpose, and surest way of providing this protection is to use a prepared query (assuming you are using the much simpler php PDO extension. the php mysqli prepared query programming interface is overly complicated and inconsistent, resulting in either a lot of extra typing for each query or in a bunch of slow code if you write a single-point general-purpose prepared query function/method.) another advantage to using prepared queries are for the few cases where you need to execute the same sql query multiple times within an instance of your script. by preparing the query once, then just executing it with different sets of input data, you will save about 5% of the execution time for most, straight-forward, queries.
  2. OR if you use the same named hidden field for all the forms, 'action' for example, each with a unique value, you can eliminate any isset() tests for that field since it will be set if the request method is post.
  3. this code is filled with mistakes (the incorrect INSERT query, a form field type = 'company', a changing/non-existent variable name) and the symptom of it seemingly working at one point, then not at another, is due to some of those mistakes and the changing data being tested that doesn't mean what you think. by testing for an empty string '""/empty() in the logic, you cannot tell if the data exists but is empty or if the data doesn't exist at all. the way to initially SELECT and retrieve data to be edited is to define an empty array variable, $post for example, before the start of the form processing code, copy the submitted form data to this variable inside of the form processing code, then after the end of the form processing code, if the variable is empty query for and retrieve the existing data and assign it to this variable. use the contents of this variable when outputting the values in the form fields. next, if the accounts db table is the primary user table, i.e. a row will exist if the user exists, then the only query in this edit form processing code should be one UPDATE query (i suspect you have repeated this code and query for each form field.) if this is instead an add-on db table, designed only to hold profile information, where there many not initially be a row for any user, than the query you have in this edit form processing code should be one INSERT ... ON DUPLICATE KEY UPDATE ... query, with the user_id being defined as the unique key that triggers the duplicate key part. your form processing code should also validate the submitted data, storing validation errors in an array variable, then only use the submitted data if it is valid. if you have repeated the posted code/query(ies) for each form field, you should instead just have one consolidated form processing code with one query that operates on all the form fields at once. you should also use prepared queries when supplying data to an sql query statement (no matter how good you think your sanitizeString() function is, there are hackers out there with libraries of injectable sql that can probably get past it, especially if you have not set the character set that php is using to match your database, and you should only apply any sort of sql protection to the data being supplied to a query, not to data that is being output in form field values and you shouldn't be trying to strip slashes and certainly not from data after you have retrieved it from your database.)
  4. it would be nice if you posted your code that's setting the $search variable (you could be doing something that you think is correct, but isn't) and your code using $result (you could be doing something that you think is correct, but isn't.)
  5. mysqli_fetch_fields() doesn't do what you think. it fetches information about the fields. it doesn't fetch data and you would have been getting php undefined index errors from your code to alert you to the problem. you need to ALWAYS have php's error_reporting set to E_ALL and when learning, developing, and debugging code, have display_errors set to ON and when on a live/public server have display_errors set to OFF and log_errors set to ON. you would want to use mysqli_fetch_assoc() to fetch the data.
  6. and did you reload the page in your browser so that the change would take effect? i just tried your form page and it displayed the value that i entered in the form in the sql query statement. next, you should NOT put external/unknown data directly into an sql query statement. you should use a prepared query, with a place-holder in the sql query, then supply the actual data when the query gets executed.
  7. your form is using method='get'. your php code is trying to use post data. if your code had checked the value in $refcode before using it, you would have already known this (it should be a null since the input variable is not set.)
  8. password_hash() is used when you store the hashed password, during registration. to test if the entered password corresponds to the hashed value, you must retrieve the hashed value from the database table and use password_verify() also, the only thing you should store in the session variable is the user's id, from an auto-increment column in your users table. to get any other user information, query for and retrieve it based on the user id.
  9. this sounds like a typical CRUD assignment and literally 'go to the next record' isn't exactly what you want because you would not be frequently entering/editing all the records in turn. don't you really want to be able to enter/edit the email address for specific/selected record(s) and you want to be able to easily go to or select the records where an email address needs to be entered/edited? if so, what you need to do is come up with a User Interface (UI) that supports what you are doing. you can do this at least four different ways - 1) display a page of the records inside a single form, where you would enter/edit any amount of email addresses at one time, then submit the form and UPDATE all the records. an enhancement to this would be to either have a check-box or use javascript to detect a change in the email field and set a value in a hidden field, for each record that indicates that the record has been changed. the form processing code in this case would use the check-box/hidden field and only UPDATE the records that have been changed. 2) display a page of the records with each as a separate form. you would enter/edit an email address and click the submit button for that record. the page would re-display after updating that record and you can enter/edit more email addresses. 3) display a page of the records with an edit link next to each one, containing the record id. when the edit link is clicked, you would retrieve the data for that record and populate a form with the existing data. when the form is submitted, you would UPDATE just the record for that id. 4) a mix of method #1 and #3. display a page of the records with an edit check-box next to each one, inside of a single form. check the boxes for the records you want to edit and submit the form. the next page would retrieve the data for all the records that were checked and dynamically output a single form with a field for each record. the single form would be submitted and processed as in method #1. the method you choose depends on how many email addresses would typically be entered/edited at one time, to provide the simplest user interface (fewest number of clicks with the smallest chance of putting data into the wrong record), and per what your assignment is to use. method #2 is probably the best from a user standpoint - identify and click on a record you want to edit, enter/edit just one email address at a time, click the submit button next to the email form field, deal with any errors from that submission, repeat for the next email address to be entered/edited.
  10. your 'set' of prices should be in an array (arrays are for sets of things.) you can then write a call-back function containing the money_format() statement and apply it to every element in the array by calling array_map().
  11. i recommend that you review the replies you have received in the thread, particularly this one - https://forums.phpfreaks.com/topic/307650-fetch_assoc-select-option-value-filter-column/?tab=comments#comment-1560580 it lists two security holes with the way you are doing this now and suggests a simple and secure method of doing this so that it works.
  12. there's no need to be editing your code when you switch the environment it runs in. if you remove the try/catch logic you have now, and let php catch the exception, it will use its error_reporting, display_errors, and log_errors settings to control what happens with the actual error information. when learning, developing, and debugging you would display all errors. when on a live/public server, you would log all errors. the only time your code should catch and handle a database exception is if your code needs to detect and handle the insertion/update of duplicate data, which is a recoverable application error, not a fatal database error.
  13. you have a php syntax error. the try/catch syntax is incomplete. you are not seeing any errors since your code never runs when there is a php syntax error. you should set the error_reporting/display_errors setting in the php.ini on your system so that php will report and display ALL the errors it detects. in most cases, you should not catch exceptions in your code (only when detecting duplicate data being inserted/updated.) you should normally let php catch and handle any exception. next, the variables being used in the connection don't appear to exist, the PDO extension doesn't have a bind_param() method (it does have a bindParam() method), the two variables you are binding don't appear to exist, and instead of binding input data,you should just supply it as an array in the execute() method call. also, if there are no affected rows, that's not a query error and using $conn->error, which isn't a PDO property anyway, won't contain any useful information. if there is a query error, an exception will be thrown and the exception handler will be executed.
  14. the biggest issues with your code (some of which have already been mentioned) are - 1) all the form processing code isn't inside the conditional statement detecting if a form has been submitted, so the part where the sql query is at can be executed when there is no post data to use. all the form processing code needs to be inside the conditional statement and if this is the only form processing code, you should instead detect if $_SERVER['REQUEST_METHOD'] == 'POST' 2) sqlsrv_errors() returns an array. you cannot echo an array. to dump the errors for debugging purposes, either use var_dump() or print_r(). also, your code needs only execute the rest of the database code if $getResults is a true value. the rest of the errors are 'follow on' errors because the query failed. you should also have some error handling for the database connection so that you only execute the query if the connection worked. 3) after you have detected that the form has been submitted, you must validate all input data before using it. this is doubly important if the form data specifies column/table names for an sql query statement, since no amount of escaping of the values (it's not string data and it's not being used in a string context) or using a prepared query (you can only supply data values, not column/table names via prepared query place-holders) will protect against sql injection. for column/table names, you must either validate that each value is only and exactly a permitted column or table name or you must use some sort of id mapping, where you submit id values instead that get mapped to column/table names internally in the code so that only id values that have a valid mapping can be used.
  15. your code should pass a minimum of information through the form, because you must validate ALL form/external data before using it. your current method also exposes all the users email addresses and probably allows someone to submit any email address to your form processing code and it will get used as the email address to send to. you should only pass the user_id through the form, then query for the corresponding email address in the form processing code. also, any data you store or update based on the form data only needs the user_id.
  16. that's because you are unconditionally assigning that string to $result and it comes after the assignment of the success message. remove the An error occurred ... line of code. the exception error handling takes care of producing a query error message.
  17. not for the case of a query error. you have an exit; statement in your code and the rest of the code on the page, where you are echoing it, is never executed. you need to remove all the try/catch logic you currently have and let php catch the exceptions, where it will use its error_reporting, display_errors, and log_errors settings to control what happens with the actual error information. when learning, developing, and debugging, you should display all php errors. when on a live/public server, you should log all php errors. the only useful time you should catch and handle a pdo exception in your code is if you are detecting duplicated data being inserted/updated.
  18. you can zip up your files and attach them to a post in this forum. because this is an execution/data problem, it will take having all the code that is needed to reproduce the problem. the issue is the forum software probably won't let you post 6000+ lines of code in a reply.
  19. because it's an indication that the code isn't correctly handling input and it's taking up unnecessary resources detecting and handling execution errors, which a hacker can then learn information from since they are being output to the client. the OP should actually be logging all php errors when on a live/public server.
  20. browsers and other http clients can/do make extra requests to pages for different reasons and your code must deal with them. it's likely that your post method form processing code is receiving a get request that it should ignore. is your code detecting that a post request was made before running any of the form processing code? as to the error you are getting, for a likely extra get request, it sounds like the variable is being initialized to an empty string or perhaps being assigned an empty string rather than having an array element added to the variable.
  21. separation of concerns. if you use his programming practice, converting the database specific code from using a non-prepared query to a prepared one, or even converting it to use the php PDO extension, is easy, because all the database specific code, that knows how to query for and retrieve data, would be groped together and be located before the start of the html document. the way to implement this separation of concerns, after you rearrange the code to get the database specific code out of the html document, is to simply fetch the data from any data retrieval query into an appropriately named php variable(s), then simply use the variables(s) (test if it's empty or not or loop over it) in the html document. once you do this, all you have to do when converting and testing the database specific code is make sure you produce the same data in the php variable(s). you won't have to touch any of the code in the html document.
  22. the OP is also executing the prepared query, fetching all the data from it, but not using that data, then executing the query again as a non-prepared query (in the foreach ... statement). also, there's no JOIN condition in the query, so every row between the two tables (withrunontablenames) are being cross joined together, and there are no integers in php/mysql that are 100 digits in length. you are also apparently (based on the query and the code after the end of the loop) using a loop to fetch what you expect to be a single row of data. if you came up with this code from a tutorial it wasn't from ONE working tutorial. you need to make use of the php.net documentation when learning the basics. you should also disable emulated prepared queries, set the character set, and set the default fetch most to assoc when you make the connection.
  23. this is a common symptom when a redirect changes either the http vs https protocol, the www. vs no www. host-name in the url, or the path after the domain in the url and the session id cookie parameters are not set up to match variations in those values. you need to insure that the the return URL you have set up in the payment gateway matches where the session was created and/or set the session id cookie parameters to match all variations in the url, rather than just the one where the session was created. these settings are covered in the php.net documentation.
  24. there's no code posted in this thread to examine and as requinix stated, these error don't have anything to do with something not being saved.
  25. before you go beating on a keyboard adding bespoke code to handle each form field, there are some functional/user things you need to fix first. then, since you have a number form fields to process, you should be dynamically processing them, not writing out lines and lines of code that have to be changed any time you need to fix a common problem or make a change to how the code works. some problems - 1) the name of each form field must be unique (you will only receive the data from the last same-named field) and since there are selectable sections, the name should also uniquely identify the section and field within that section. 2) all the <label></label> tags need a corresponding id='...' attribute in the form field they correspond to. there's only one form field with an id attribute now and it doesn't match the for='...' attribute in its label. 3) if they are not already on the same page, both the form processing code and the form should be on the same page. this will let you display any validation errors when you re-display the form and you also need to repopulate the form field values with any previously submitted data so that the visitor doesn't need to keep selecting/reentering data, which will increase the possibility of typo mistakes. 4) except perhaps during testing when you are entering your own email address, these emails are NOT being sent from the email address that someone enters in the form. they are being sent from the mail server at your web host (even if the receiving mail server is the same one.) the From: email address must either have a domain name the directly corresponds to the sending mail server or there must be an SPF DNS zone record at the domain name being put into the From: email address that indicates your sending mail server is authorized to send email for that domain name. short-answer: the From: email address should be the same as the To: email address and you only put the entered email address into a Reply-to: mail header, after validating that it is only and exactly one correctly formatted email address. 5) all submitted values that get put into the email message need to have htmlentities() applied to them to prevent any injected css/javascript from being operated on when the message is read. even though you are not intentionally creating a html email (now), email clients can be configured to treat all emails as html. also apply htmlentities() to any data values that you output on the web page to the visitor. 6) you also need to test for errors from the mail() (or even better, one of the php mailer classes) and report success or failure of the attempt to send the email. 7) you should use an array to hold the validation errors. this will let you store each error with the corresponding field as a key. this will let you test at any point of there is or is not an error with any field and it will also let you display the errors next to the corresponding field. as to the php code you have now, one of the points of programming is to NOT develop carpal tunnel syndrome. if you find yourself writing out discrete variables, copying variables to other variables, and writing/repeating lines of code that only differ in the piece of input data they operate on, you need to find a different way of processing the data. btw - the clean_string() function in the code now is pointless, ridiculous copy/pasted code from the web. get rid if it. the way to process a set of data like this is to create a data structure (array) that defines the expected form fields, what validation tests to preform on each one, and if they are required or not. you would then loop over this defining data structure and validate each input in turn. at the end of the validation logic, you would just test if the array holding the validation errors is empty or not. if it is empty, you know all the data is valid and you can use it as input to the email logic. this kind of dynamic processing will also help simplify or eliminate logic for the different sections/subject. you can just have a field in the defining data structure with the corresponding subject value, then use the selected subject value to only operate on fields having the corresponding value. the form processing code will then just end up needing to do - 1) detect that a post method form was submitted 2) validate the input data 3) if there are no validation errors, use the input data 4) if there are validation errors, display then when you re-display the form and also repopulate the form field values.
×
×
  • Create New...

Important Information

We have placed cookies on your device to help make this website better. You can adjust your cookie settings, otherwise we'll assume you're okay to continue.