-
Posts
4,207 -
Joined
-
Last visited
-
Days Won
209
Everything posted by Jacques1
-
Yeah, good luck finding “help” with that attitude.
-
The mysql_* functions are long dead and will be removed from PHP in the near future. Manual SQL escaping is generally obsolete, we use prepared statements now. With prepared statements, you don't have to worry about SQL injections -- as long as you pass all values to the statement parameters and never insert them directly into the query. Note that if you use the PDO database extension, you have to set the PDO::ATTR_EMULATE_PREPARES attribute to false to get actual prepared statements. Otherwise PDO will only do a kind of auto-escaping (which is much less secure).
-
I just told you what's wrong and what you need to do. Those blue underlined words are links, you need to click on them. If you have specific questions about the links, feel free to ask. But we can't do the reading for you.
-
None of this makes a lot of sense. First of all, how did you come to the conclusion that there's an encoding-related security issue? Who told you that? Where did you read that? Secondly, the script doesn't really do anything, so how would an “attack” even look like? We cannot “attack” hypothetical code which isn't actually there. There is a problem in your code, but that has to do with the total lack of any HTML-escaping. If you just dump the user input into the HTML document, then of course anybody can inject malicious code.
-
SHA-1 is awful for hashing passwords. Even an old graphics card can calculate around 3 billion(!) SHA-1 hashes per second, so finding out the passwords of your users is trivial. And, no, “salting” doesn't help. Whoever told you that SHA-1 is an acceptable password hash algorithm either doesn't know what he's doing or lied to you on purpose. Your code is also wide open to SQL injections, because you happily dump the user input into the query string, allowing anybody to write their own queries. Never heard of Little Bobby Tables? So you have quite a lot of work ahead of you. The first thing you should do is avoid crap resources and stick to professional programmers who actually know PHP. For example, Padraic Brady is a well-known security expert. See also this thread about password hashes.
-
Yes, I understand that. But the question was how your GUI looks like. Is there a “ban” button next to each user in the list?
-
You select the user by their ID (whatever that is). How exactly the ID is passed to the server depends on your GUI. Is there a “ban” and “delete” button next to each user in the list? Is there an extra page for editing a specific user? Either way, be very careful about Cross-Site Request Forgery. If you just accept any request coming from an admin, then it's easy for a malicious website to forge requests and ban or delete arbitrary users. You need to include a special random token in each request (see Synchronizer Token Pattern). You also shouldn't actually delete users, because people may click the button by accident. There's also a big risk of ending up with broken references. Instead, mark the user as deleted.
-
It's still the old problem: If a user chooses a name like “admin” or “payment”, you're in trouble, because now they suddenly control “official” domains like admin.mysite.com or payment.site.com. The “public” prefix in front of it doesn't really help. The point is that the user sites are isolated in a separate subdomain. Of couse it doesn't have to be called “sites”. Choose anything which sounds good and makes it clear that those are user-defined sites. Even then you should not write insecure code. Make security a habit.
-
While you fix your code, you also need to remove brainfarts like this one: die("Connection failed: " . $conn->connect_error); This prints internal database errors directly on the screen for everybody to see, which is obviously not the best idea. Not only are cryptic error messages very irritating for legitimate users. It also helps attackers gain detailed information about your system. Internal error messages are for the developers (you), not the users. They belong into a log file. And the funny thing is: That's exactly what PHP does if you configure your php.ini appropriately. So no need for any weird die() statements.
-
Never ever invent your own security algorithm. You cannot win, especially when you don't happen to be a genius cryptographer. And indeed this is horribly insecure. I'm not sure why you think that chaining two extremely weak algorithms somehow magically creates a strong algorithm. It doesn't. Both MD5 and SHA-256 allow an attacker to try billions(!) of passwords on an average PC. At that rate, a few passwords more or less simply don't matter. The attacker can always buy or rent better hardware. And who knows if this specific kind of chaining creates certain cryptographic weaknesses? Cryptography is an exact science, you can't just randomly throw together algorithms. The salt generator is also very broken. This weird procedure only produces around 58 “random” bits (a common recommendation is 128 bits). Even worse, the bits aren't really random, because the str_shuffle() function was never designed for security purposes. It's based on trivial data like the current server time and the process ID which are easily guessable. Long story short, always use established solutions. The state-of-the-art for password hashing is the bcrypt algorithm. If you have PHP 5.5, you can use it through the new Password Hashing API. If you have at least PHP 5.3.7, you can use the password_compat library which emulates the API. Older PHP versions are defective. No. This library is hopelessly outdated and seems to have been abandoned by the author. It also comes with a lot of compatibility baggage which can lead to security issues (for example, there's a fallback to an MD5-based algorithm if bcrypt isn't found). See above for an up-to-date solution.
-
There seems to be a whole lot of misunderstandings. The function utf8_encode() function expects an ISO 8859-1 encoded string (read the manual!) and transcodes it to UTF-8. I'm fairly sure this is not what you want. If your data already is encoded with UTF-8, the function will fail miserably. And if your data actually is ISO 8859-1, it doesn't make a terrible lot of sense to convert it to UTF-8, because this only increases the data size. A typical cause for database-related encoding issues is that the connection isn't set to the right encoding. It's not enough to declare the database as UTF-8. You also have to tell MySQL that all outgoing data should be encoded as UTF-8. Otherwise it will use the default encoding which is usually “Latin1” (a variant of ISO 8859-1). So you need to take care of three encodings: The encoding of the database itself (you seem to have already done that). The encoding of the database connection. This depends on which database extension you use. PDO? MySQLi? The old ext/mysql? For PDO, the encoding is set in the constructor, MySQL has mysqli_set_charset(), and the old extension has mysql_set_charset(). Last but not least, you have to declare the HTML document as UTF-8. This should not be done with a meta element but within the Content-Type header: Content-Type: text/html; charset=utf-8
-
Do it step by step. This problem obviously involves several tasks, the most basic one is to make an HTTP request with PHP. Can you do that? If not, read up on cURL. When that's done, read the chapter about cookies so that you can maintain the session cookie after the log-in.
-
Like always, exotic solutions indicate that there's something wrong with the concept itself. Session IDs do not belong into the URL, especially when it's the session of the admin. There's a reason why any halfway competent admin immediately turns off URL-based sessions in the php.ini: URLs parameters are typically written to the server log, they may even be transferred to arbitrary websites through the referrer, and there's a huge risk of people accidentally sharing the secret data. If the preview should (only) be available to the admin, then why is it a separate subdomain of mysite.com? It should be a subdomain of the user's admin domain (or simply a path if the preview is safe). This also makes sense from a logical point of view: The preview is a special service of the admin site. This way the preview is simply covered by the normal admin session. No authentication tricks required. Unfortunately, this is somewhat hard to implement with your current domain scheme. It might make sense to change it to something like this: public.joe.sites.mysite.com admin.joe.sites.mysite.com preview.admin.joe.sites.mysite.com Of course that means you have to put all domains behind a generic *.mysite.com catch-all and lose the possibility to handle them differently at DNS level. But that shouldn't be a problem. Besides, the code is rather awful. While we discuss about security details, you just dump the user input into your HTML markup with no escaping whatsoever.
-
Before you revolutionize permission management, you should read up on database normalization. You repeat all data of a permission for every assigned entity. This means you may get conflicting information. Even worse, there's no definite set of permissions: You may accidentally end up with different spellings of the same permission, or the assigned permission may be plain garbage data which isn't implemented anywhere in the application. The ID can point to two entirely different entities, which means you cannot check if it actually makes sense at database level (unless you fumble with complicated workarounds). If you fix this, you'll actually end up with three tables: one for the permissions, one for assigning permissions to users, and one for assigning permissions to groups. That's already way more complex than the “classical” approach. Yes, your model makes it easy to add/delete a permission or select all permissions of a user. But if you want to check if a user has a specific permission, things get cumbersome: Instead of a plain SELECT some_permission FROM users WHERE user_id = ..., you now need something like SELECT COUNT(*) > 0 FROM user_permissions WHERE permission = 'some_permission' AND user_id = .... More complex checks may also involve joins between the various tables. The question is what exactly you want to achieve. What's the concrete benefit of your model for your application?
-
That's a very, very bad idea. In fact, there's something wrong with your server configuration if your application can use this query. The LOAD DATA INFILE query requires the FILE permission. Granting this to your application means that it can both read and write external files. It's easy to see that an SQL injection attack may now compromise the entire server: The attacker just has to grab critical passwords from your filesystem or create a malicious script. Sure, the Unix account of the MySQL database should not have unlimited access to the filesystem. But if the database server is already misconfigured, I wouldn't bet on that. Long story short: You do not want LOAD DATA INFILE. If it's active, turn it off. Then use a plain INSERT query.
-
That looks good. However, a subdomain can set a cookie for a parent domain. So bob.public.mysite.com is able to create a cookie for .public.mysite.com which would get sent to all user sites as well as www.mysite.com. This comes with several problems: If you fail to regenerate the session ID, it's possible to perform a session fixation attack: Bob would simply set a custom PHPSESSID cookie for .public.mysite.com and wait for the victim to log in with the known session ID. If you keep the anti-CSRF token in a cookie, an attacker may be able to overwrite it with a known value and circumvent the protection. Anybody can start a session and make another visitor resume it. Imagine the following scenario: I log in under my account on this forum and plant the session cookie on you. Then you write a private message to somebody, not realizing that you're logged in as me. I'm now able to retrieve your private message from my own message folder. There's no defined precedence for cookies which only have different domains, so your session cookie may actually be overriden by a “fake” cookie, leading to a denial-of-service attack. Some of this can be fixed, some problems you simply have to accept. In general, pay extra attention to any cookie-related features, especially the sessions.
-
Like I said, there's no need for any testing whatsoever. If the database connection fails, there's not much you can do about it. So you let PHP go through the standard procedure for fatal errors: The error message is written to the logfile, and the script is aborted (the webserver should of course display a pretty error page). The only reason for catching an exception is to fix the problem or go through a special error procedure for this particular issue. In any other case, just let PHP do its job. I know it's very popular in the PHP world to clutter the code with try-catch statements or even inform the user about the specifc problem (“the database connection has failed”). This is nonsense. The default error handling procedure is much smarter than what most people implement: It sends detailed error information (message, location, stack trace) to the appropriate device according to the php.ini. During development, you want the error messages directly on the screen; on a live site, you want to write them to a log file. It aborts the script in case of a severe error. It emits a 500 status code, allowing the webserver to serve an appropriate error page. What's the point of overriding this procedure with a dumb echo $error_message? Now you lose a lot of important information (the message is not enough), you show internal information to the user, you keep the script running when you probably shouldn't, and you lie to the webserver by sending a 200 status code.
-
And what are we supposed to do now? None of us is psychic, so if you want help with your code, you need to actually show us that code.
-
You'd use isset($db) instead of checking for $db directly, because the former will not trigger any notices. However, the whole code is rather poor and a good example of what you should not do. Catching exceptions to print the error message is not only nonsense, it's downright harmful, because anybody will see your internal database issues. Don't do that. Just leave the exception alone, then PHP will trigger the standard error procedure (as defined in the php.ini) and abort the script. No need for any checks or custom error handling.
-
Yes, you'll probably want a blacklist to at least block the obvious abuse. Ideally, the user sites shouldn't be on your main domain at all, because any official-sounding subdomain will be attributed to you. But I understand that the whole purpose of the project is to host the sites under your name.
-
This makes absolutely no sense. The part after the “<<<” is the delimiter for the heredoc/nowdoc block. It simply tells PHP where the block begins and where it ends. You do not use variables or anything like that. Just make up an identifier: <?php $comment = $_POST['comment']; $str = <<<'I_AM_THE_DELIMITER' Example of string spanning multiple lines using nowdoc syntax. I_AM_THE_DELIMITER; If you want to insert some variable, you have to insert it into the block between the delimiters: <?php $comment = $_POST['comment']; $str = <<<I_AM_THE_DELIMITER This is the user comment: $comment I_AM_THE_DELIMITER; Note that you need a heredoc if you want to directly insert variables like in a double-quoted string. A nowdoc doesn't do that (as I already explained).
-
Not telling the user if a particular name exists is easier said than done, and most people get this wrong. For example, what about the “I forgot my password” feature? In this case, the username or e-mail address is the only input. What if the name doesn't exist? Do you just keep quiet and let the user wait forever for their e-mail? Then they'll be pissed and won't come back. It's also possible to derive information from subtle factors like time differences and errors. For example, you don't check the password at all if the name is already wrong. So a wrong username will abort the script immediately, whereas a wrong password will make the script run much longer. That alone tells me whether or not a name exists, even if you don't explicitly say that. It may also be possible to provoke errors and use the feedback to get information. What if I try to register a name which is already registered? If that leads to an error, I'm again able to tell if a user exists. Long story short, it's very difficult if not impossible to hide usernames. You may try it so that it's at least harder for an attacker to get the information. But the actual solution is to not make the usernames secret in the first place. In that sense, you should not use the e-mail address as the name but rather let people choose arbitrary public names.
-
There seems to be some confusion. The whole point of the “long URLs” is to put the user sites into a separate namespace so that people won't mistake them for your content. Imagine a user choosing a name like “login”. In your model, they'll get the domain login.yourdomain.com and may fill it with any content, which is obviously a big problem (it would allow them to perform the perfect phishing attack). To make a clear separation between your content and the content of your users, I suggested the user-sites subdomain. Obviously you don't want that. Well, then don't use it. But this means you have to manually check every single registration and make sure people won't choose confusing or “dangerous” names (“joe” would be OK, “payment” probably not). Either way, using the “short domains” as some kind of alias for the long ones would cirumvent the whole purpose.
-
The above code is insecure. When you call session_start(), then PHP will either start a new session or resume an existing session. So you may very well run into an old session which is already filled with other data. If you just use that, you'll easily run into trouble. For example, the old session may have contained special permissions which are suddenly transferred to the user who has just logged in. One might also imagine a scenario where an attacker starts a session, fills it with data and then plants it on the victim. This way the attacker controls the content of the victim's session. So it's not enough to generate a fresh ID. You also have to delete all content to make sure you actually get a fresh session with no previous data: <?php session_start(); // generate a fresh ID and delete the current session if (session_regenerate_id(true)) { // clear any previous session content $_SESSION = array(); // write the new data to the session $_SESSION['email'] = $email; } I strongly recommend that you regenerate the ID before you write any data to the session. The old ID may be known by an attacker, and if the session_regenerate_id() call somehow fails or isn't called, then you end up writing all data to the attacker's session -- which is a classical session fixation attack. By the way, the logged_in parameter is superfluous, because you just have to check the presence of the email parameter to find out whether or now the user is logged in.
-
Be aware that heredocs are interpreted like double-quoted strings, so character sequences like $x or \n will be interpreted in a special way. If you want plain text like in a single-quoted string, use a nowdoc: <?php $str = <<<'MYTEXT' Example of string spanning multiple lines using nowdoc syntax. MYTEXT;