Jump to content

Jacques1

Members
  • Posts

    4,207
  • Joined

  • Last visited

  • Days Won

    209

Everything posted by Jacques1

  1. Hi, note that the approach itself is incorrect: If a name isn't taken yet and two users ask for it at the same time, they're both allowed to have it. That's because there's a gap between the check and the action. While the name may not have been taken when you've checked, it may very well be taken when you insert the new row. Unique values must be enforced at database level with a UNIQUE constraint. In your application, simply try to insert the row, and if it fails due to a violation of the constraint, you know that the username is already taken.This also lets you get rid of the extra query. When you've chosen your database extension (I strongly recommend PDO instead of MySQLi), we can show you how to do it.
  2. Do not use $_REQUEST. This mixes all URL parameters, request body parameters and cookies into one big array, which can easily lead to collisions and a lot of confusion. Explicitly fetch the values from the right source. In your case, you want $_POST. Using isset() makes no sense, because even if the user leaves the fields empty, the parameters are still set. What you want is empty(). This checks if the parameter is not set or has a falsy value. In addition to that, use the required attribute on the fields so that the user doesn't have to wait for your response to see that the input is wrong.
  3. Unfortunately, you can't just do a SELECT and then check for duplicates in your application, because this doesn't work with concurrent requests: User A and user B both try to insert the same new value at the same time. Your application checks the table. Since the value isn't present yet, both users are allowed to pass. Now you have a duplicate despite your check. But I do agree that you should reconsider your decision and the database design. Does this whole thing really make sense?
  4. Hi, yes, there has been a change of how MySQL processes joins: But the real problem is that you've mixed those syntaxes in the first place. Either use the old syntax with commas or the new syntax with “LEFT JOIN”, “INNER JOIN” etc. When you put the two together, you're asking for trouble.
  5. Hi, “doesn't work” isn't very useful as a problem description. What is the problem? Are you getting an error message? An unexpected result? The infamous white screen? Something else? You need to tell us, because we can't see your screen from here. Either way, the code has a couple of serious issues: Plaintext passwords? Seriously? Even if you don't give a damn about the website, that's just unacceptable. You need to hash the passwords with a strong algorithm like bcrypt. If you have PHP 5.5, the functions are already built into the language. Otherwise, there's a compatibility library. What is this weird strip_tags() doing there? It makes absolutely no sense. It's actually downright harmful, because it mangles the user input. If the user chooses, say, "2$<53Ab!.-" as their password, your strip_tags() truncates it to "2$", because it happens to have a "<" in it. Oops. Why on earth do you put the passwords in the session? Shouldn't you protect them rather then throwing them around? The plaintext passwords don't go anywhere. You store a hash of them in the database, and that's it. Do not put them into the session, a cookie or whatever. The mysql_* functions you're using are obsolete since more than 10 years and will be removed in the future. Nowadays, we use PDO or MySQLi.
  6. Let me Google that for you. No offense, but has it never occured to you that other people may have parsed XML files before you? And that they have created tools and tutorials for exactly this purpose?
  7. What? Are you saying you don't want any value of the two fields appear again in any of the fields? So in this example, the 1 would be a duplicate:? +---+---+ | a | b | +---+---+ | 1 | 2 | | 3 | 1 | +---+---+
  8. I gave you two links which explain the basics of accessing a database and hashing a password in a very simple way with plenty of examples. I suggest you read them and try things out yourself. If there's something particular you don't understand, simply ask. But I can't do the learning for you.
  9. Hi, this code is pretty weird. I think you should fix it before you do anything else. How is this a login? I don't see you doing anything with the session. I hope you don't use the cookies for authentication? Because those can easily forged by anybody, so the first thing people will do is take over the admin account. The mysql_* functions you're using are obsolete since more than a decade and will be removed in the future. Nowadays, we use PDO or MySQLi. Using SHA-512 to hash passwords is completely ineffective. An average gaming PC can easily calculate hundreds of millions of SHA-512 hashes per second and find out almost any password simply by trying out a lot of combinations. You need a hash algorithm specifically designed for password protection. What are all those strange random numbers and cookies supposed to do?
  10. The session lifetime of PHP is not a timeout. This is a misunderstanding. When you set the lifetime to, say, 15 minutes, it does not mean that sessions only last 15 minutes. It means that if a session isn't accessed for 15 minutes, the next session garbage collector run may remove it. The garbage collector is invoked with a certain probability (typically 1%) by session_start(). That means there's no definite time limit for the sessions. They can last forever if the user constantly accesses them. Otherwise, they will be removed at some point in time depending on the probability setting and the number of requests you get. The lifetime is merely a cleanup mechanism for the session storage. It doesn't help you at all in your case. If you want a session timeout, you need to implement it yourself as you did above.
  11. Hi, the server needs to perform a full TLS handshake. Implementing this on your own is practically impossible and simply not a good idea. So there are basically three options: You use a WebSocket library which can do TLS (looks like Wrench is a good choice, but I've never tried it myself). You rewrite the code from scratch using the high-level Streams. This is what Wrench does under the hood. You use nginx as a reverse WebSocket proxy to convert the WSS traffic to plain WS.
  12. I understand what you're saying, I just don't agree with it. To me, caching the status in the session in order to save one trivial query per request (maybe not even that, because the script may already have a query on the users table) is a classical premature optimization. I have absolutely no reason to believe that this has any relevant performance benefit whatsoever. If Timothy provides us with a concrete profile or benchmark which indicates the opposite, I'm happy to change my mind. Until then, this is pure speculation, especially since none of us has any information about the website. Is this a private homepage with 10 users per day running on a cheap shared host? A commercial site with thousands of requests per second? Yes, you can do this. It's acceptable. But when I implement something, it should be more than just acceptable. So what are the benefits? It may theoretically save us a fraction of a millisecond under certain circumstances. Frankly, I don't care. Checking the status is slightly easier, because you can just get it from the session. But you might as well wrap the database stuff in a function or method. Compared to the downsides (duplicate data, risk of running into weird situations), I don't find this very convincing. Again: Both is valid. This is not some life-altering decision. But when I have to choose between clarity and some speculative micro-optimization, I always go with clarity. The arguments against TLS are rather weird in my opinion. When you're dealing with the passwords and e-mail addresses of other people, then this is sensitive data. It's not always about money. How do you come to the conclusion that attacks against plaintext traffic are “extremely unlikely”? This certainly depends on how exactly each user accesses the Internet. If you have TLS enabled, you let them make that decision instead of forcing HTTP on everybody. Worrying about the performance or possible costs is, again, speculative. If we're talking about a big site, then, yes, you'll have to take this into account. But who said anything about a big site? As far as I'm aware, Timothy might as well run some small personal website, in which case there would be zero costs and zero performance issues.
  13. I think the message should be clear to anybody who uses a computer: The file you're trying to include does not exist (or at least it can't be read). Are you sure you got the path right? You're claiming the library script is in this folder: /public_html/amember/library/Am/ Is it? I highly doubt that you have a folder called “public_html” in the root directory. Also, your own script is under this folder: /home/tradeco/public_html/ That's a very different “public_html”. I think you actually want this one.
  14. Like I said, you'll need to read up on transactions. The procedure is as follows: You start a transaction (in serializable mode). You read the current balance of the sender and at the same time lock the row so that concurrent transactions won't be seeing the same (now obsolete) value. If the balance is sufficient, you subtract the amount from the sender. If this UPDATE succeeded and has affected rows, you add the amount to the receiver. If this UPDATE also succeeded and has affected rows, you commit the transaction. In any other case, you roll it back. Note that the table must use the InnoDB engine, because MyISAM doesn't support transactions. As a quick demonstration: <?php $database_options = array( PDO::ATTR_EMULATE_PREPARES => false, PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION, PDO::ATTR_DEFAULT_FETCH_MODE => PDO::FETCH_ASSOC, ); $database = new PDO('mysql:host=localhost;dbname=test;charset=utf8', 'NAME', 'PASSWORD'); // test data $remitterId = 2; $remitteeId = 1; $amount = 100; $database->query('SET TRANSACTION ISOLATION LEVEL SERIALIZABLE'); $database->beginTransaction(); try { $transactionSuccessful = false; // get current balance $remitterBalanceStmt = $database->prepare(' SELECT balance FROM users WHERE user_id = :user_id FOR UPDATE -- this locks the row '); $remitterBalanceStmt->execute(array( 'user_id' => $remitterId, )); $remitterBalance = $remitterBalanceStmt->fetchColumn(); if ($remitterBalance === false) { echo 'Invalid remitter.<br>'; } elseif ($remitterBalance >= $amount) { // add amount to remittee's balance $addAmountStmt = $database->prepare(' UPDATE users SET balance = balance + :amount WHERE user_id = :user_id '); $addAmountStmt->execute(array( 'user_id' => $remitteeId, 'amount' => $amount, )); if ($addAmountStmt->rowCount()) { // subtract amount from remitter's balance $subtractAmountStmt = $database->prepare(' UPDATE users SET balance = balance - :amount WHERE user_id = :user_id '); $subtractAmountStmt->execute(array( 'user_id' => $remitterId, 'amount' => $amount, )); if ($subtractAmountStmt->rowCount()) { $transactionSuccessful = true; } else { echo 'Failed to subtract amount.<br>'; } } else { echo 'Failed to add amount.<br>'; } } else { echo 'Insufficient funds.<br>'; } if ($transactionSuccessful) { $database->commit(); echo 'The transaction was successful.<br>'; } else { $database->rollback(); echo 'The transaction failed.<br>'; } } catch (PDOException $transferError) { $database->rollBack(); echo 'The transaction failed due to technical problems.<br>'; } Do not copy and paste this code. Use it to understand the idea and then write your own script.
  15. Hi, this is clearly an encoding issue. Looks like your browser (or wherever you got this from) misinterprets UTF-8-encoded data as ANSI text. This is a typical problem when you forget to set the character encoding of the HTML document.
  16. Since the OP explicitly asked about admin sessions, I think we all agree that this does not fall into the category of low-security applications. If he had asked about a different situation, then, yes, this would be a different discussion. My point is simply that things like using HTTPS and hardening the PHP sessions are relevant for everybody and should be the default until there are good reasons against them (like in your case). Personally, I don't like terms like “highly sensitive data” or “high-security website”, because it makes people think that their own data and their own website aren't important enough to be protected. But we actually need much better security on average websites.
  17. Checking the user ID separately doesn't make a lot of sense and is kind of missing the point. The point is that the whole transaction is only valid if there was no error of any kind in the two actions. Errors include invalid user IDs, insufficient funds, database problems etc. Unfortunately, you cannot do this with PHP alone. Well, you can try, but it's not gonna work reliably, and people may exploit this for cheating (transfer money that does not exist etc.) If you're serious about your game and neither want broken data nor cheating, you'll have to take a very different approach using database transactions. The whole transfer must be atomic (all or nothing), and different transfers running at the same time must not interfere with each other. This does require some knowledge and rigor. So do you want the correct (but hard) way? Or are you OK with the game working only partly?
  18. What the manual is saying is that you're not supposed to erase the $_SESSION variable itself, because PHP needs this for the session mechanism to work. But you can and should overwrite the variable with an empty array during logout: $_SESSION = array(); Otherwise, $_SESSION will still hold the old data while the script runs. This may confuse later parts of the code and make it assume the terminated session is still valid. So logging out a user really consists of three different steps: session_destroy() deletes the session file on the server $_SESSION = array() clears all session data in the running script asking the client to delete the session cookie If you forget the latter, the client will keep sending you the old ID, and PHP will reuse that ID for the next session. This isn't necessarily a problem as long as you properly renegerate the ID in the login procedure. But it's still somewhat unclean. I strongly disagree with the statement that you can ignore session security, and I find it sad that people still believe TLS/SSL is only for banks. How many more hacks do we need until everybody realizes that they, too, are affected by threats? Without TLS, you have absolutely no way to tell who got your data, what server you're talking to and whether the page you received is authentic. Anybody who happens to be between you and the server may read, intercept or manipulate the communication. Do you really wanna use your admin account in this environment? Yes, you may simply hope that everybody arounds you plays nice. But I don't think this is the right attutide for running a website. The same is true for session security: If you don't take care of security, then attackers will exploit that as soon as they find it worthwhile. It's not always about big money. It could simply be a script kiddie trying to impress some friends. Or maybe somebody is angry at you. Security is crucial, no matter if you're running a bank or a small homepage. Both are at risk. And, really, there's simply no excuse for not securing your site. Getting a free TLS certificate from StartSSL, including it in Apache and updating the php.ini is a matter of minutes and doesn't require any money or special knowledge.
  19. Hi, you cannot mix the new MySQLi functions with the old mysql_* functions. Those are two entirely different extension from different times. I wonder how you even got this idea, because I see people trying this again and again. You either need to use MySQLi (which is recommended) or stick to the old extension which will be removed in the future. But you can't have both.
  20. Hi, it's true that users cannot edit the session data, but storing the user role in the session is still a very bad idea, because it leads to redundant data. You now have two places where you store the information: the database and the session. If they fall out of sync, you have a problem. For example, a user who is no longer supposed to be an admin may still have a session with the admin status set to true. Or the other way round: You've just granted a user admin access, but they still run around as normal users until they log out. How did you even get to this idea? Is this some kind of micro-optimization thing? Simply grab the data from the database.
  21. Take a close look at the erroneous line. You have a variable, then a comma, then a string with a comma in it and immediately after this a number. This is not valid PHP syntax. As bsmither already pointed out, you probably wanted an empty string followed by a comma rather than a string with a comma in it. That is, you want '', rather than ',' See the difference?
  22. What's that lonely comma string doing there? Is it a separate method argument? Then you need a comma after it. Is it supposed to belong to the time? Then you need string concatenation. But you can't just put it there with no context.
  23. Do you have any reason for setting this option at runtime? If you've just copied it from that website without understanding what it does, remove it.
×
×
  • 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.