-
Posts
4,207 -
Joined
-
Last visited
-
Days Won
209
Everything posted by Jacques1
-
You should use prepared statements. Manual escaping with mysqli_real_escape_string() is too error-prone (as you can see), and stripslashes() has nothing to do with security. It's a leftover from the early days of PHP when “Magic Quotes” still existed.
-
Yeah. It's great that you want to share your knowledge, but sometimes you should just get to the point and answer the question instead of rambling on for an entire hour. Then we don't have to argue about how to interpret your text. I really appreciate your expertise, but reading your replies is, excuse my French, fucking painful. And I don't think that's a language problem. By the way, I was specifically referring to this statement: Sounds pretty clear to me: You recommend against testing each individual field with isset(). And I strongly disagree with that. My point is that incomplete submissions do happen and should be treated like any other input error (display a proper error message, emit a 400 code etc.). You cannot rely on your forms, because sometimes form fields get blocked by browser plugins (as in my example) experienced clients may send the data with cURL or a user script rather than submitting your form (which is perfectly legitimate) the client may have cached on old version of the form All those cases should lead to a proper error message, not a pile of PHP warnings while you try to “validate” fields that don't even exist. OK? If you still disagree, try to make technical arguments. My English sucks? Meh.
-
I strongly disagree with the opinion that you can just assume that all fields are present. This is simply not the case. For example: In a different PHP forum, I had the problem that I was suddenly unable to write messages. Since the server only displayed generic errors, nobody had any idea what's going on. I took me several days to figure out that the problem was caused by a missing form field which should have been rendered by JavaScript but was blocked by my NoScript plugin. That's a lot of trouble only because the server failed to properly check each form field. It's also perfectly legitimate if the user decides to submit the data directly rather than through your form. I wonder why this is still seen as something “suspicious”. It's not. So do check the presence of each field, and do emit proper error messages. Don't make any assumptions regarding the user input.
-
How about you run the code and see for yourself? Actually, none of your example will do what you expect, because you don't seem to understand the difference between the assignment operator “=” and the equality operator “==”. What you want is something like this: <?php if (isset($_POST['query']) && $_POST['query'] == 'php') { // do this } else { // do that } Note the isset() check. Without this, $_POST['query'] may not even exist and trigger an error.
-
Sure, this could be implemented. It would basically turn the session ID into a nonce. The problem of this approach is that it tends to favor attackers (who are fast) over legitimate users (who are slow). A realistic session hijacking attack is automated. That is, the attacker will grab the current unused session ID, immediately exploit the session and then dispose it. So when the legitimate user finally clicks on some button, makes another request and invalidates the shared chain, the attack is already over. Even worse: Now the user is logged out and cannot do anything until he logs in again (which is hopefully still possible after the attack). So this session chain really only helps against long-running (manual) attacks. This would certainly stop a script kiddie who grabs a session ID and then tries to exploit it through trial-and-error. But it won't stop an attacker who knows what he's doing. And of course the implementation is much more complex than the standard session mechanism. The approach is certainly valid, and I've seen similar suggestions (with less sophistication). But I'm not sure if it's worth the trouble. In any case: Nice idea.
-
What do you mean by “XSS filters”? Concurrent sessions are not bad. I use multiple sessions on multiple PCs all the time, and it would be very annoying if I had to constantly log-in and log-out. So before you implement a security feature, you need to actually weigh its effectiveness against its impact on usability. Limiting the user to a single session doesn't really increase security, but it massively decreases usability. So that's something I wouldn't do (except maybe for very special cases). There are many different ways of using the Internet, and you have to be careful not to make false assumptions. For example: If a user makes one request from one continent and then a second request from an entirely different continent, that may look “suspicious” at first. You may even be tempted to terminate the session. But what if the user has simply switched to a different proxy or VPN? That's perfectly legitimate.
-
It's a bit difficult to understand what you mean, but it seems you're talking about session hijacking where an attacker finds out the victim's session ID and then takes over the session. Your session IDs should already be purely random, because that's what PHP does by default. The standard PHP session IDs look something like this: luu4aldc1jjsie2mmq7n8vb650 If your application uses something like the name instead, then it's very broken – but I don't believe you've actually overriden the session ID mechanism. The default session IDs are hard to predict but not perfect. To make them truly unpredictable, it's recommended that you use session.entropy_file and session.entropy_length to mix the output of a strong random number generator like /dev/urandom into the IDs (16 bytes are sufficient). However, unpredictable IDs don't help you when the attacker has found an XSS vulnerability. Now they can either “steal” the session cookie (unless you've set the HttpOnly flag), or they can simply trigger actions on behalf of the user. You're basically screwed. Long story short: Double-check for XSS vulnerabilities and consider using Content Security Policy. XSS will defeat any session protection, so you absolutely must take care of this problem first. Make the session IDs unpredictable with a random number generator like /dev/urandom. Always generate a new session ID after the user has logged in (this actually prevents session fixation rather than session hijacking). HTTPS on the entire site is a good idea, because it prevents the session ID from being sent across the globe as plaintext. Mark the session cookie as HttpOnly so that it cannot be read by JavaScript. Terminate the session in case of inactivity and after a fixed amount of time (e. g. 1 hour). But make sure to save the user input so that it won't get lost. Make sure the session IDs are only transmitted and accepted via cookies. That is, enable session.use_only_cookies and disable session.use_trans_sid. Binding the session to the IP address or the user agent doesn't really help. The UA is trivial to guess, and IP addresses are often shared, reused or even available to everybody (think about public proxies or VPNs). This binding may even cause more harm than good, because some users change their IP address regularly. It that automatically logs them out, they have a problem.
-
PDO is bindParam replaced?
Jacques1 replied to greenace92's topic in PHP Installation and Configuration
Don't copy-and-paste random code snippets you found somewhere on Internet. Using sleep(60) causes the script to hang for a entire minute (or triggers a timeout), which is something you certainly don't want in a web application, neither during development nor in production. -
Yeah, that's what I meant by “nonsensical code”. Personally, I'd scrap this stuff and start over, this time with valid PHP code.
-
PDO is bindParam replaced?
Jacques1 replied to greenace92's topic in PHP Installation and Configuration
When you pass an array to the execute() method, then those elements will be bound to the parameters. That's simply how PDO works. It's also possible to explicitly bind variables/values to parameters using either PDOStatement::bindParam() or PDOStatement::bindValue(). The advantage of explicit binding is that you can declare the type of the value. By default, all values are passed as strings, which means MySQL has to cast them if they aren't actually strings. This works most of the time, but sometimes type casting has unexpected results. -
The best way to find out is to try it.
-
First of all: How about you generate sensible error messages which actually allow you (and your users) to figure out the problem? This would make life much easier. Large parts of your code are also nonsensical. For example, you have a random mixture of mysql_* functions and mysqli_* functions. Don't you realize that those are two entirely different PHP extensions? And why do you HTML-escape all input before inserting it into the database? This will leave you with tons of “garbage data” full of HTML entities. I strongly recommend you learn the basics of PHP before you write any serious application. As to your original problem: Are you sure you want to check for $_POST['submit']? Because the value of your submit button seems to be “Modify Data”. To avoid this kind of confusion in the future, I recommend you simply check if the request used the POST method: if ($_SERVER['REQUEST_METHOD'] == 'POST') { ... } Then you don't have to rely on the value of some obscure submit button.
-
Call to a member function beginTransaction() on a non-object
Jacques1 replied to PineSmokes's topic in Applications
You shouldn't do anything with the database connection. Setting it to null at some point is very confusing and error-prone, because if you happen to need the connection afterwards (e. g. to roll back a transaction or log data), you'll again run into “non-object” errors. PHP itself will terminate the database connection after script execution has ended, so no need to do that manually. You shouldn't use “magical strings” like "true" either, because this again leads to confusion and bugs. Yes, PHP's sloppy type system will accept expressions like !"true", but it won't necessarily do what you expect. For example, !"false" also evaluates to false, even though you probably wanted true. Use actual booleans, and use meaningful variable names like $registration_successful rather than a generic $ok (which could mean pretty much anything). Even better: Separate the error handling from the rendering of error message. In your application code, you only collect the errors that happened. And at the very end, you render the message and send an appropriate response code. Don't forget the HTTP code! While most of us are good at processing purely visual information, this isn't true for every client (consider a visually impaired human or an automated client). <?php // possible errors const ERROR_EMAIL_ADDRESS_EXISTS = 0; const ERROR_EMAIL_TRANSMISSION_FAILED = 1; // ... // collect all errors in a set $registration_errors = []; $response_code = 200; // example error: violation of UNIQUE constraint of email column $registration_errors[ERROR_EMAIL_ADDRESS_EXISTS] = true; $response_code = 400; // set the HTTP response code http_response_code($response_code); ?> <!-- your HTML markup --> <?php if (array_key_exists(ERROR_EMAIL_ADDRESS_EXISTS, $registration_errors)): ?> Sorry, but your e-mail address is already registered. Do you want to <a href="password_reset.php">reset your password</a>? <?php endif; ?>- 8 replies
-
- pdo
- begintransaction
-
(and 3 more)
Tagged with:
-
Call to a member function beginTransaction() on a non-object
Jacques1 replied to PineSmokes's topic in Applications
Now you're setting the $db variable to null after you've printed the test result. That's a bad idea, because now $db definitely isn't a PDO connection. Remove the assignments and try again. PHP has a password hashing API which hides all the low-level cryptography behind a few simple functions. To hash a password, all you have to do is call password_hash(): <?php // password hash parameters; put this into a separate configuration file const PASSWORD_HASH_ALGO = PASSWORD_BCRYPT; // bcrypt is currently the only choice const PASSWORD_HASH_COST = 14; // adjust this to your own hardware (hashing a password should take roughly one second) const PASSWORD_MAX_LENGTH = 56; // bcrypt has a maximum input length of 56 bytes $test_password = 'zkWMfOmSTPS4wY8C8BzCaG'; // create a hash from the plaintext password if (strlen($test_password) <= PASSWORD_MAX_LENGTH) { $password_hash = password_hash($test_password, PASSWORD_HASH_ALGO, ['cost' => PASSWORD_HASH_COST]); echo 'The password hash is: '.$password_hash; } else { echo 'The password must not be longer than '.PASSWORD_MAX_LENGTH.' bytes.'; } To verify a password against a hash, you just have to call password_verify(): <?php $password_hash = '$2y$14$5ue3w80sIho.B5GjqlppB.nwwnpUL3WN8re5peY3sRJ.w5idlgmKC'; $test_password ='zkWMfOmSTPS4wY8C8BzCaG'; if (password_verify($test_password, $password_hash)) { echo 'The password is correct.'; } else { echo 'The password is incorrect'; } Because this function mangles the input in a primitive attempt to remove HTML tags. This is nonsensical and will lead to data corruption. For example, the perfectly valid string “1 < 3” will be truncated to “1 ” just because it happens to contain the “<” character. Never change the user input. Store it as is. When you need to insert the data into a critical context, use escaping (e. g. htmlspecialchars() for HTML contexts). There's no need to clutter your code with try statements and stuff like echo $e->getMessage(), because PHP itself is perfectly capable of printing error messages. Simply enable display_errors in your PHP configuration and set error_reporting to -1. Now all unhandled errors will be printed on the screen. If you want the message to be nicely formatted, you can use the Xdebug extension. When the code goes into production, you turn display_errors off and enable log_errors instead. This is much safer than having to remove tons of try statements, because there's no risk of overlooking one of those debug statements and accidentally revealing information about your server. In general, you should leave exceptions alone and let the PHP error handler do its job. Catching an exception is only needed if you have a specific solution to this specific error, which is rare. Most problems cannot be solved at runtime.- 8 replies
-
- 1
-
- pdo
- begintransaction
-
(and 3 more)
Tagged with:
-
That looks a lot more like Ajax. So, problem solved? If you still have trouble with multiple methods, open the link I gave you. Your problem has already been solved. Of course you could also try to implement your own Ajax interface. But in my experience, most homegrown APIs are very bad. For example, why does your delItem() method return this weird status array? All it should do is update the database (and maybe throw an exception if that fails). Sending status codes around is the job of a separate class which takes care of the client/server communication.
-
I doubt that your code works with anything. If I understand it correctly, then you don't use Ajax at all, you just call $user->delitem($delItem) in your main script and somehow insert the return value into the JavaScript code. This has nothing whatsoever to do with Ajax. Again: You need a separate(!) PHP script which processes the Ajax request. There's no other way. That's simply how Ajax works.
-
I'm not sure if you actually understand Ajax, because the code you've posted doesn't really make sense. The idea is this: After the client has received a page of your website, they use JavaScript to make an additional HTTP request to a PHP script. The target script receives the request, processes the data and takes action. For example, it might instantiate your User class and call the delItem() method. It's also possible to send data back. Upon receiving the HTTP response, the JavaScript code checks for errors and processes the data (if present). So, no, you can't just send an Ajax HTTP request to an arbitrary URL. Your server needs to provide a PHP script which understands your Ajax requests and processes them. You can either implement this all by yourself. Or you can use an established protocol like JSON-RPC so that you don't have to mess with low-level HTTP stuff.
-
You should use http_build_query() and HTML-escaping instead of relying on string gymnastics: <?php /* Your PHP code goes here */ function html_escape($raw_input, $encoding) { return htmlspecialchars($raw_input, ENT_QUOTES | ENT_SUBSTITUTE | ENT_HTML5, $encoding); } // test data $your_row = [ 'id' => 42, 'ompCreatedBy' => 'foobar', ]; $your_url = 'kf_orders_entered_detail.php?'.http_build_query(['id' => $your_row['id']]); ?> <!-- Your HTML markup goes here --> <tr> <td><a href="<?= html_escape($your_url, 'UTF-8') ?>"><?= html_escape($your_row['ompCreatedBy'], 'UTF-8') ?></a></td> </tr> Not only is this much clearer than messing with PHP strings. It's also much more secure and reliable, because you won't end up with syntax conflicts or even attacks.
-
Call to a member function beginTransaction() on a non-object
Jacques1 replied to PineSmokes's topic in Applications
Well, PHP disagrees. Are you sure you've actually included the database script in the main script? You should use version control software like Mercurial or Git. This allows you to save snapshots of your code and restore previous versions in case you accidentically mess up your application. You mean a separate table, right? The problem with storing the token in a separate table is that you need two queries instead of one, and you additionally need a transaction to turn the two queries into a single logical action. Why make things so complicated? There's a one-to-one relationship between the token and the member, so you can and should simply put the token into the members table. The code you're learning from is relatively good compared to most other PHP tutorials, but it's far from perfect. Many parts are nonsensical or even harmful (like using strip_tags() or printing database errors on the screen or relying on some homegrown password hash mechanism). So I don't think it's a good idea to just copy and paste the entire code. Learn from multiple resources, take everything with a grain of salt, and write your own code.- 8 replies
-
- 1
-
- pdo
- begintransaction
-
(and 3 more)
Tagged with:
-
Call to a member function beginTransaction() on a non-object
Jacques1 replied to PineSmokes's topic in Applications
Your $db variable is not a PDO connection. Maybe the variable name is wrong, maybe you haven't even included your database script, maybe the connection failed but you keep going nonetheless. By the way, is there any reason why you need to store the activation token in a separate table? The code would be a lot easier (no second query, no transaction), if you stored the token right in the members table.- 8 replies
-
- 1
-
- pdo
- begintransaction
-
(and 3 more)
Tagged with:
-
It might actually be a good idea to sit down for a couple of days, figure out what your task is and then come back with a coherent problem description. You keep telling us about BLOBs and databases, but that's not the task. It's your personal approach to the task (which could be completely wrong). Do you understand the difference? If you want us to help you, then we need to know the task, because that's the only way to figure out the right approach. It seems what you actually want is move an uploaded file to a target destination while preventing name collisions. So is that the task?
-
You're not escaping the user input. You do have this weird testStr() function, but you only use it occasionally. Most of the time, you just dump the user input straight into the queries: $update = mysql_update("users", array("password", "salt", "p_prompt"), array($password, $salt, 1), "email = '".$_POST["email"]."'"); ^^^^^^^^^^^^^^^ And I'm fairly sure your friend does not want his server to be compromised. You mean besides the total lack of proper input handling? Have you checked the PHP error log? What does it say? If it doesn't say anything, what does it say after you've enabled error reporting and logging? Look, we have no magic debug sense. If you insist on keeping your code, then you will have to narrow down the problem. We cannot do that, because we don't have access to your server, we don't have your code, we don't have your database, we don't even have a problem description. All we know is that the password appearently isn't updated, which could be caused by just about anything. Personally, I'd throw away the code and start from scratch, this time with a proper password reset workflow, proper queries, proper password hashing, proper error handling where you call trigger_error() for every mysql_error(), proper mailing. This sounds like a lot of work, but it's probably faster and definitely more fun than debugging the stuff you currently have.
-
shaddf, what is your actual task? Forget about this weird power failure stuff (WTF?), and just tell us what you need to do. In your previous thread, you said you want to move files from any directory to another. Then rename() is the answer, plain and simple.
-
How about the MySQL manual? The ENUM type can be used to define a fixed set of possible values. For example, a “gender” column might be declared as ENUM('male', 'female', 'other'). But be aware that the ENUM type is very inflexible and rather confusing. You cannot easily add new values, you cannot add meta data, and MySQL accepts incorrect values which are then replaced with empty strings (unless you're using strict mode). Often times, it makes more sense to use a foreign key pointing to a separate table instead of an ENUM.