Jump to content

Jacques1

Members
 View Profile  See their activity

  • Posts

    4,207

  • Joined

  • Last visited

  • Days Won

    209

 Content Type 

Everything posted by Jacques1

  1. Jacques1
    Yeah, “just be careful”. We know how well that works. Unfortunately, programmers are not always as smart and careful as they think they are. Just recently, a fellow PHP developer had to load server-side translation strings into JavaScript. The strings don't come from the user, so the super-smart programmer figured that he didn't have to follow our standard practice of escaping all dynamic data. Things indeed went well – until we switched to French. The French use a lot of apostrophes in all kinds of places, and that's a problem within single-quoted strings. Long story short: Large parts of the application blew up, and our super-smart programmer began to understand why rules are sometimes a good idea. Of course you're free to make your own mistakes. But I think every programmer should eventually realize that they aren't infallible and that “just be careful” just isn't good enough.
  2. Jacques1
    Yes, the CSS and JavaScript parsing comes after the HTML markup has been parsed. In plain HTML, script and style elements are actually identified in a very odd way. You don't even need a proper closing tag. A “</script ” (note the space at the end) is sufficient to terminate a script element.
  3. Jacques1
    You should always write proper code and treat every dynamic value as if it was dangerous. Yes, sometimes you may get away with a buggy implementation. But why take the risk? <?php $data = array( 'foo' => 42, 'bar' => 123, ); ?> <!DOCTYPE HTML> <html lang="en"> <head> <meta charset="utf-8"> <title>JSON test</title> <style> .hidden { display: none; } </style> <script src="http://code.jquery.com/jquery-1.11.1.min.js"></script> <script> $(function () { var data = JSON.parse($('#my-data').text()); console.log(data); }); </script> </head> <body> <div id="my-data" class="hidden"><?= htmlspecialchars(json_encode($data), ENT_QUOTES | ENT_SUBSTITUTE, 'UTF-8') ?></div> </body> </html> Note, however, that this is not the recommended solution and should only be used if Ajax is defnitely not an option.
  4. Jacques1
    Do not turn off slash escaping! This is a very important security feature which prevents cross-site scripting attacks. The PHP developers haven't implemented this just for fun. Try this code with standard JSON encoding: <?php $json = array( 'foo' => '</script><script>alert(/XSS/);</script><script>', ); ?> <script> var foo = <?= json_encode($json, JSON_UNESCAPED_SLASHES) ?>; </script> Nothing bad happens. Now the same thing with JSON_UNESCAPED_SLASHES as suggested by Ch0cu3r: <?php $json = array( 'foo' => '</script><script>alert(/XSS/);</script><script>', ); ?> <script> var foo = <?= json_encode($json, JSON_UNESCAPED_SLASHES) ?>; </script> Congratulations, we now have a cross-site scripting vulnerability, because any occurence of the term “</script>” within the JSON data terminates the current script context and allows the user to create a new one with arbitrary JavaScript code. So this is not a solution. It doesn't even address the actual problem. The problem is that ginerjm takes some PHP value and drops it right into a script element. While that may seem like the obvious thing to do, it's really an awful approach which can lead to all kinds of security vulnerabilities and bugs (as we just saw). Never generate JavaScript code. Instead, make a PHP script which serves the JSON-encoded data and then fetch it with JavaScript. jQuery even has the getJSON() method which automatically decodes the response: $.getJSON('image_data.php', function (image_data) { // image_data is the already decoded array }); If you absolutely must embed the data into your HTML document, then JSON-encode it, HTML-escape it, put it into a hidden div element and parse it with JavaScript using JSON.parse().
  5. Jacques1
    Do you not realize that anybody can create the log-in cookie herself and skip your “protection” entirely? You might as well check the URL for “is_admin=true”. Sessions do not expire when you close the browser. The session ID is in fact stored in a cookie, and you can set the lifetime to anything you want. The point is: Unlike your home-made cookie stuff, PHP sessions actually work. We've all used them for many years. Wouldn't it make sense to rely on a proven solution rather than trying to do it all by yourself?
  6. Jacques1
    Help you with what? You neither posted the code nor described the problem. Remember: We're not sitting next to you, we don't see what you see. Right now, all I can say is that your database layout is broken. So your first steps would be to learn the basics of normalization and then restructure your database. Maybe that also solves your current problem (whatever that is).
  7. Jacques1
    Even the dumbest script kiddie knows how to create a cookie. The code is simply nonsense, no matter how low your standards may be.
  8. Jacques1
    Did you read the code you're using? Do you realize that anybody can create any cookie they want? The entire “password protection” is nothing but a giant brainfart, so no wonder it “doesn't work” (whatever that means). Learn PHP and write your own code. It's no rocket science. In this case, you'd simply use standard PHP sessions to store the log-in status.
  9. Jacques1
    First of all, forget about the performance stuff. Some members of this forum have developed a certain fetish for randomly coming up with dubious micro-optimizations, but that shouldn't bother you. Worry about readability, not “performance”. You do not use fetchAll() together with a while loop. Like the name already says, this method fetches all rows in one go. The $row variable is simply a misnomer by jazzman1. Since the return value is the entire result set, so you would name it $addresses or something like that. You should generally avoid meaningless variables like $row or $data. What “row”, what “data”? Last but not least, always fetch associative arrays so that you can reference the columns by their name. I have no idea why the others used numerical arrays (maybe the fetish again), but you don't want that. I can assure you that it's friggin' annoying if you constantly have to go back to the query to figure out what on earth $row[12] actually means.
  10. Jacques1
    First of all, this has absolutely nothing to do with “destroying a session”. All you want to do is unset a particular value of the $_SESSION array. The unset statement does exactly that. However, you can't just unset your own $sessions[$name] variable, because then you only delete this variable and not the original data. You need to actually unset $_SESSION[$namespace][$name].
  11. Jacques1
    PDO is much more comfortable than MySQLi. For example, fetching data with a prepared statement only requries prepare(), execute() and a foreach loop. The same thing in MySQLi requires prepare(), bind_param(), execute(), bind_result() and a while loop to call fetch(). But more importantly, PDO is a universal database interface, it's not limited to one particular database system. If you decide to switch from MySQL to PostgreSQL, you can do that without having to rewrite your entire code again. If you want to use an embedded SQLite database, you can access it in the same way you access your main database.
  12. Jacques1
    You fetch the row as a numerical array, extract the values and put them into an associative array? Why not fetch an associative array in the first place? Or rather: Why not simply fetch the entire result set? That's one line of code for everything: $result = mysqli_fetch_all($result, MYSQLI_ASSOC);
  13. Jacques1
    First of all, PDO::quote() in the context of a MySQL database actually is mysql_real_escape_string(). They both call the exact same MySQL library function. The only difference is that PDO::quote() also wraps the result in single quotes. Much has been said about prepared statements already, and it's great that you're planning to use them in the future. However, many people aren't aware why they're worth the extra effort. Why not just escape the values? The problem is that escaping is highly susceptible to errors. There are not only the obvious mistakes like forgetting to escape a value, forgetting the quotes, making wrong assumptions about “safe” values etc. You can also run into much nastier issues. Try out the following code, for example: <?php /* CREATE TABLE users ( name VARCHAR(255) PRIMARY KEY, password VARCHAR(255) NOT NULL ) CHARACTER SET big5 ; INSERT INTO users SET name = 'admin', password = 'hunter2' ; */ $db = new PDO('mysql:host=localhost;dbname=test', 'YOUR_USER', 'YOUR_PASSWORD'); // never use SET NAMES to change the character encoding! $db->query('SET NAMES big5'); // user input $_POST['name'] = 'admin'; $_POST['password'] = "\xbf\x27 OR 1-- "; $user_stmt = $db->query(' SELECT name FROM users WHERE name = ' . $db->quote($_POST['name']) . ' AND password = ' . $db->quote($_POST['password']) . ' '); $user = $user_stmt->fetchColumn(); var_dump($user); We clearly escape the input with PDO::quote(), so an SQL injection should be absolutely impossible. Yet this is exactly what happens: The user is able to log-in as the admin by performing an SQL injection through the password parameter. The problem is that escaping depends on the character encoding of the database connection. In order to find critical characters like the single quote, MySQL must know how they're actually represented on byte-level. The encoding information is set to the default encoding (usually latin1) when you connect to the database, and it's updated when you change the encoding with the charset parameter of the PDO constructor. However, what what we do above is use a SET NAMES query. This also changes the character encoding of the connection, but it does not update the encoding information. So PDO::quote() still thinks you're using latin1 when you've actually switched to big5. This mismatch makes the function blind and breaks it entirely. Fortunately, many character encodings used today (ASCII, ISO 8859-1, UTF-8 etc.) are more or less compatible with each other, so you usually get away with encoding bugs. But I wouldn't rely on this. In any case, the example shows how fragile escaping is. Note that what PDO calls “prepared statement” is by default just automated escaping and has all the issues outlined above. If you want actual prepared statements, you have to explicitly set the PDO::ATTR_EMULATE_PREPARES attribute to false.
  14. Jacques1
    But now you know, right?
  15. Jacques1
    No, you should not have a unique key over the user ID and the transaction ID. RTFM, for heaven's sake. That's where the info is. PayPal will send you various status updates with the same transaction ID (and obviously the same user ID). This is how the system works. If your database rejects the “completed” message because it already has a “pending” message for the same transaction ID, you have a serious problem. I wish you lots of luck. You'll need it.
  16. Jacques1
    “attribrute” in line 71. I recommend that you start using an IDE (integrated development environment) like Netbeans or Eclipse. It will detect obvious mistakes while you write the code and save you a lot of time. The code highlighter in the forum is behind the “<>” button.
  17. Jacques1
    No offense, but are you sure you're ready to take actual money from actual people? Appearently you haven't even bothered to read the manual and get a basic understanding of how IPN actually works. Receiving an IPN message does not mean you should randomly add the amount to some account. PayPal will send you all kinds of status updates, and, yes, you may receive duplicate messages. Besides that, the code you've posted is wide open to SQL injections. You don't even seem to be aware that your application must be protected. So that's where you will manage the accounts of your users? And anybody with basic SQL knowledge is free to change any account to any balance? I strongly recommend that you forget about this project for now. People are willing to accept mistakes as long as it's all just for fun. But when their money is involved, that's usually when the fun ends.
  18. Jacques1
    The question is: What do you want to do? We can philosophize about theoretical solutions to theoretical problems all day long, but I don't think it will help anybody. What matters is the concrete case. That's how you decide whether or not a specific approach is useful. So what are those errors? What scenario do you have in mind?
  19. Jacques1
    By printing a heading with the first letter whenever the first letter changes.
  20. Jacques1
    Well, the question is: What do you want to do? What's the purpose of catching the exception? Besides that, always catch specific exceptions, avoid Pokémon Exception Handling as demonstrated above. When you just catch all exceptions, you'll quickly end up swallowing important errors which you never intended to handle.
  21. Jacques1
    Cool.
  22. Jacques1
    ... which is what I just said. What's your point? What are you trying to tell us? Did you want to remind us that MySQL supports SELECT queries with no clauses? Thanks, but this is neither new nor relevant here. The problem is that iarp has forgotten the FROM clause. Of course you can, try it out. Or rather: What makes you think it's not possible? PDO doesn't care where the value comes from.
  23. Jacques1
    That code makes absolutely no sense. You're asking for the IV size of some very exotic encryption algorithm and a mode which doesn't even support an IV (ECB). This should actually return 0, but for some reason it returns 32 instead. That's far too much. You just need to choose appropriate length for the random string. The fact that this function was originally meant to generate initialization vectors is irrelevant here. It might as well be called mcrypt_random_bytes(). That would actually be a better name. You don't need to set up an image subdomain on your local test server. That would be a bit silly. You only need this when the site is actually online. I don't know what you're read about cross-domain communication, but this is only a problem if you want to load the images via JavaScript. Is that the plan? If you just want to include the images in your HTML document, then of course you can load them from any domain you want.
  24. Jacques1
    You do realize that the WHERE statement is actually important here, right?
  25. Jacques1
    Regarding the previous code: This indeed looks a lot better and clearer. There are still some issues and oddities, though. What is the client supposed to do with the internal error code? In fact, some of the codes are clearly not meant for the end user: If you're having trouble with the temporary folder or a PHP extension, that's none of their business. You should have proper error handling with a message that's actually useful and doesn't expose internal information (e. g., “The file is too big. We only allow file sizes up to ... KB.”). Also consider sending an appropriate HTTP response code like 422 Unprocessable Entity. Wherever you've copied the Mcrypt code from, that's not what I meant. MCRYPT_RIJNDAEL_256? We're not doing any encryption here. You just call mcrypt_create_iv() with a fixed size of 16 bytes and MCRYPT_DEV_URANDOM as the source. Do not use MCRYPT_RAND. This is even worse than the uniqid() stuff from before. The MIME type for JPEG images is image/jpeg. There is no such thing as “image/jpg” (like I already said above). What does the ominous escape() function do? Escaping for what? This should get a proper name, and I'm pretty sure it will turn out it's not appropriate for file paths. The ctype_digit() check is good, though. The “invalid file” message should be a bit more descriptive: List the allowed types again. There are still issues with the indentation. It doesn't matter if you use tabs or spaces, but be consistent. Always use the same amount of whitespace for one indentation level. The lonely exit statement in line 38 should be replaced with proper error handling. I think this is overkill. Unless you expect large amounts of traffic, just store the images on your server.
×
×
  • 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.