Jacques1
-
Posts
4,207 -
Joined
-
Last visited
-
Days Won
209
Everything posted by Jacques1
-
You skipped the external jQuery script I posted above, but that's actually important: <script src="https://code.jquery.com/jquery-1.11.1.min.js"></script> This goes into the head element before the other script. Of course you're free to use plain JavaScript instead. You also can't have multiple elements with the same ID. Use a class. Then you have a strange definition of “working”.
-
They've decoded a similar script at Web Hosting Talk, and it was clearly malware. So, yes, that's most probably an infection. Do not try to manually “clean” the server by removing those code snippets. You never know what is still lurking somewhere else. Instead, take the site offline, change all admin credentials (passwords, SSH keys etc.) and look for obvious backdoors like new user accounts. If you can find out how the attack happened, that's of course helpful, but don't spend too much time on it. After this, you basically have to backup your current data, nuke the server, reinstall it with a secure(!) configuration and then carefully restore the website. Check for security updates of Zen Card. Of course this is easier said than done. Do you have a friend who can help you? Anybody who knows how to manage a webserver?
-
I don't want to disturb your happy debugging session, but why on earth do you fix a feature when you already know that it's wrong? You've already spent 5 hours on this MD5 crap. And when you're done, congratulations, you can throw it all away and start over with an entirely different interface. Learning to hash passwords with MD5 is like learning to write websites for Netscape Navigator 1.0: It's not very useful in the 21st century. Of course you're free to keep debugging. Maybe you like it. But if your goal is to get your application done, then it's time to stop playing with fossiles from the 90s and get serious: The Password Hashing extension.
-
That link alone doesn't really tell us anything. Create a minimal example of the problem so that we can actually try it out. Also, did you check the JavaScript console like I suggested? And of course you have to escape all values, not just the one that goes into the data attribute.
-
Did you try this exact code? Then the function is definitely called – unless you've played with your browser settings and turned off JavaScript or something. So the actual question is: What did you do to break the code? Since I'm not psychic, I cannot answer this until you actually post your script. If possible, create a minimal example which we can try out. Then we don't have to speculate about possible problems. It's also a good idea to check for error messages in the JavaScript console of your browser. You usually have to hit F12 to open the developer tools and then go to the JavaScript tab. // I realized that I only gave you the resulting HTML markup. The underlying PHP script is this: <?php $row['track'] = 'http://example.com/some.track'; ?> <!DOCTYPE HTML> <html lang="en"> <head> <meta charset="utf-8"> <title>Passing PHP values to JavaScript</title> <script src="https://code.jquery.com/jquery-1.11.1.min.js"></script> <script> $(function () { $('#track-link').click(function () { alert('This is the track: ' + $(this).data('track')); }); }); </script> </head> <body> <a id="track-link" href="#" data-track="<?= htmlspecialchars($row['track'], ENT_QUOTES, 'UTF-8') ?>">Download</a> </body> </html>
-
You can't just drop a PHP value into a JavaScript context. Depending content of the variable, this either results in a bug or even a major security vulnerability. Adding quotes also misses the point: Maybe you no longer see the problem, but of course it's still there. You must escape the value and put it into a standard HTML context. You can use a data attribute, for example. Then fetch the track from this attribute: <!DOCTYPE HTML> <html lang="en"> <head> <meta charset="utf-8"> <title>Passing PHP values to JavaScript</title> <script src="https://code.jquery.com/jquery-1.11.1.min.js"></script> <script> $(function () { $('#track-link').click(function () { alert('This is the track: ' + $(this).data('track')); }); }); </script> </head> <body> <a id="track-link" href="#" data-track="http://example.com/some.track">Download</a> </body> </html> Do not forget the escaping! Safely inserting a value into a href attribute is even harder, but I guess that's another story.
-
There's actually a quick way to improve the code quality and maybe even solve the problem: Delete that terrible “security” script. I'm actually pretty sure this is just a prank to make fun of people who steal code from the Internet. I mean, an SQL injection vulnerability in a function against SQL injections? That can't be true. The rest is also pretty funny: So whenever the English word “by” occurs in the URL, then I'm reported as an evil hacker and thrown out, but it's perfectly fine to perform a full-blown SQL injection through POST parameters or cookies? Like I said, it's a prank. It was funny, but now it's time to become serious.
-
No, no, no. Blacklisting does not work. No matter how smart you think you are, there will always be somebody who is smarter than you and knows how to circumvent your filter. This has happened to thousands of programmers before you, and it will happen to you as well. It's an arms race you cannot win: While you keep adding new filter rules, attackers keep inventing new techniques. Actually, I've already broken your filter: echo CleanUp('<img src="#" onerror="alert(\'XSS\')">'); And if you fix that, I already know a different technique. And if you fix that, I already know a different technique etc. It's hopeless. XSS filtering like you do it is fundamentally wrong. If you want to allow a certain subset of HTML, then you need a professional library like HTML purifier and a whilelist(!) of acceptable elements and attributes. So instead of trying to enumerate every single attack technique out there, you specify what the user can do.
- 2 replies
-
- 1
-
-
- php
- strip_tags()
- (and 1 more)
-
What? Do you even know what that means?
-
The browser ignores the meta element if you've already declared the content type with an HTTP header. In your case, the element only takes effect if the user downloads the document and opens it offline. But using two different content types is of course a bad idea. I generally have no idea why you insist on this exotic XHTML stuff, but that's a different story.
-
Code review (directory handling) working, but not pretty
Jacques1 replied to anderson_catchme's topic in PHP Coding Help
Please use code tags so that we can actually read the code. Sorry, but I don't think the script is good. First of all, what's the whole point of those user folders? Are you trying to prevent filename collisions when your users upload images? That's not how an upload works. Letting people choose their own filenames is a major security risk and will lead to collisions in any way. Instead, you have to choose the name using a random number generator or an AUTO_INCREMENT column from the database (random numbers are preferable). The user-provided filename is only informational and may only be stored as metadata. And when you choose proper filenames, you don't need all those user folders. I also don't understand the purpose of appending the time. Shouldn't the usernames be unique? If they're not unique, then a simple timestamp doesn't help you, because two people may very well upload their files within the same second. Besides the conceptual issues, there are several technical problems: Using the working directory is a bad idea, because it depends on the server architecture. You could end up pretty much anywhere. So always specify the concrete path you want, don't rely on your server to figure it out for you. While you do use a prepared statement on top of the script, for some reason you've decided to go back to unescaped queries later in the script. That's obviously a security vulnerability or at least a bug. The control flow is hard to follow, because there's no proper code structure. For example, the three if statements actually belong together, but you just write them down one after another. Why do you use a LIKE query to fetch the user data? Why do you fetch the result with a loop when you only expect one row? The usernames are unique, right? ... Personally, I'd start from scratch. Before you write down any code, think about the filename problem I mentioned above. -
Ahmedamer, you can ask your question as often as you want, the answer will always be the same: The code you've stolen is garbage, PHP simply doesn't work like that. You do not store your user credentials in friggin' cookies. Why did you even create a new thread? Strider64 just pointed you to his log-in tutorial which is actually a good introduction. So why not take the chance? Learn from people who know what they're doing, not from some 10-year-old crap code you found somewhere on the Internet. I understand that you're impatient and expect to have a fully working application by tomorrow. But programming takes time, especially when you're new to all this. Don't rush it. Sure, you could take the next best code snippet you found on Google, upload it and hope that it will do what you want. But then you'll have the same problem like the people who open random e-mail attachments: You'll get “hacked”. How does that help you? Calm down, throw away the garbage code and carefully read Strider's tutorial. If you have a question about it, I'm sure we can help you.
-
Did you actually read that code? What's the comma at the end of the line doing there? What is SQLPMC.getElementById() supposed to do? SQLPMC already is the element you're looking for. I suggest you take your time and carefully write the code line by line. You also need to get familiar with the JavaScript console of your browser (just Google for it). This will show you the exact errors so that you can fix them.
-
Whenever you have to pass a dynamic value to a query, either use a prepared statement or mysqli_escape_string(). Whenever you have to output a dynamic value on your site, use htmlspecialchars(). It doesn't matter whether or not the value comes from the user. Escape all values unless they're hard-coded. Don't even try to distinguish between “safe” values and “dangerous” values. This is extremely risky, it's a waste of time, and it doesn't make sense. Even if a value is “safe” in the sense that it doesn't come from the user, it may very well cause a syntax error when you insert it into a query without prior escaping. So don't do that. When you use htmlspecialchars(), make sure to specify the character encoding and set the ENT_QUOTES flag: htmlspecialchars($input, ENT_QUOTES, 'UTF-8'); The character encoding must match the encoding of your HTML document.
-
The input event.
-
Why do you use a function when you have no idea what it does? ob_start() is not a solution. It's a workaround at best. The problem is that your header.php script already generates output while you're still doing application logic. That's bad. The output should come after you've processed the data, decided what to do, emitted Location headers and whatnot. So this is a problem of your application structure. Fix it, and the error will go away.
-
The HTML entities do not come from strip_tags(). Appearently you're also using htmlspecialchars() or htmlentities(). strip_tags() is generally a very poor choice. This function is supposed to remove HTML tags, but it's way too primitive to do that correctly. Chances are it will just mangle the entire input. In your case, for example, it removes the whole e-mail address, which is probably not what you want.
-
Both scripts are poor, but the second one is even worse than the first. First of all, you can't just dump a PHP value into your HTML markup, especially when that value comes from the user. This makes your application wide open to cross-site scripting attacks and leads to all kinds of bugs. Every value which isn't hard-coded must be escaped with htmlspecialchars(). Mixing PHP logic and HTML markup is generally poor style. It quickly turns your application into an unreadable, unmaintainable mess. One of the most important principles of modern programming is that separate aspects should in fact be separated: The PHP code is for the logic, the HTML markup is for the presentation. Both are only loosely coupled; for example, instead of generating an HTML document, you might as well create a PDF file or an Excel sheet or whatever. So what you should do is keep your PHP and your HTML separate: Put all the code on top of the script and move all the HTML to the bottom. When you're more experience, you should also consider using a template engine like Twig. Last but not least, you really need to start writing proper HTML. Maybe this was just a quick example, but invalid markup and ancient elements like font are just lame. A sanitized version of the code might look like this: <?php // The parameter may or may not be present; we need to actually check this. $color = null; if (isset($_GET['clr'])) { $color = $_GET['clr']; } ?> <!DOCTYPE HTML> <html lang="en"> <head> <meta charset="utf-8"> <title>Test</title> </head> <body> <?php if ($color): ?> <p>The color you chose is: <?= htmlspecialchars($color, ENT_QUOTES | ENT_SUBSTITUTE, 'UTF-8') ?></p> <?php else: ?> <p>Please choose a color</p> <?php endif; ?> </body> </html>
-
See the PHP site. There are quite a lot of security improvements: When you download a resource via HTTPS using file_get_contents(), PHP now actually checks the certificate. Before, certificate validation was turned off by default, which is of course a joke and left a lot of applications wide open to man-in-the-middle attacks. The Mcrypt extension no longer accepts incorrect keys or missing initialization vectors. The function hash_equals() allows you to compare secret strings without leaking their content through time differences.
-
If those are your actual database credentials, now it's time to change them. Guys, please stop stealing crap code from the Internet. That stuff is at least 6 years old, and it's absolutely horrible. I wouldn't even call it code, it's actually malware waiting to be executed by unsuspecting newbies: Plaintext passwords stored in cookies? WTF? The SQL injection vulnerabilities can be used to steal arbitrary data or take over your entire server through the database system. The cross-site scripting vulnerabilities can be used to attack your users. The inevitable MD5 hashes are just laughable given the computing power of current hardware. The entire session code is broken beyond repair. And so on ... Would you download a random executable file and run it on your PC? No? Then don't download random PHP code and run it on your server. C'mon, you can do better than this. With a little brainpower from you and help from us, I'm sure you can write your own, sane code.
-
The reason why addslashes() doesn't work is because it doesn't take the character encoding of the input string into account. It assumes that every string is ASCII-encoded, which is of course nonsense. If you apply addslashes() to a non-ASCII string, pretty much anything can happen: The function may cease to work, or it may damage the input, or it may actually yield the correct result. You never know. That's definitely the last thing you want when you're trying to protect your database. mysqli_real_escape_string() does take the encoding into account, so it theoretically works. However, it's extremely fragile: You must never forget to escape a value. You must always wrap the escaped string in quotes. You must not mess with the character encoding. For example, a simple SET NAMES query can break the function entirely. History has shown that the average programmer simply cannot do this. People forget to escape their stuff all the time, or they only escape the values which they find “dangerous” (this never works), or they forget the quotes, or they screw up the encoding. Pick any applications, and you'll probably find dozens of SQL injection vulnerabilities. So we need a more robust solution which doesn't require perfect programmers. And that's where prepared statements come into play. stripslashes() is a brainfart from the early days of PHP. It tries to solve the problem of cross-site scripting by simply killing anything which looks like HTML markup. Chances are it will destroy your entire input and leave you with a bunch of garbage data. And it doesn't even work for attributes. So strike this garbage from your memory. Cross-site scripting is an output problem, not an input problem. You cannot solve it by removing “evil data” from the user input (whatever that means). Instead, you have to escape the value when you generate the output. And this is done with htmlspecialchars().
-
Not sure if you understood my reply. I'm saying that you should stop relying on PHP to find the scripts for you and instead specify the concrete path. Like so: include '/var/www/my_website/functions/user.php'; Of course you shouldn't literally hard-code the path. You can use the __DIR__ constant to get the absolute path of the script and then go from there: include __DIR__ . '/functions/user.php'; (assuming the calling script resides in /var/www/my_website) Using HTTPS gives you an entirely separate site which may be different from the HTTP site (different PHP settings, maybe even different content). So it's not too surpring that the behaviour of PHP changes. Since I don't know your exact server setup, I can't tell you the specific reason. But is it even relevant when the entire approach is already a bad idea?
-
There's a simple solution: Stop relying on fragile include paths. Always specify the concrete script that should be included, don't make PHP guess what you mean.
-
3 randomized fields, no duplicates - buggy! NEED HELP!
Jacques1 replied to phpsyn's topic in PHP Coding Help
It's fascinating that whenever you give a PHP programmer a choice, they always pick the worst possible option. The entire task could be done in 10 lines of code and 10 minutes of your time. But, no, we don't want that. We want 100 lines of code, we want to work on it for 2 hours, and we want to turn the whole thing into a friggin' lottery: Will we be lucky this time and get three unique elements? I understand that you're new to PHP. But isn't this a matter of common sense? There's a short, simple and fast solution. And there's a long, ugly and slow solution. Now, which one is preferrable? I'm sorry for addressing you personally, this is not about you specifically. But I see this pattern over and over again, and I wonder what on earth is going on. -
There are no major differences between PHP 5.3 and PHP 5.5. The problem is that many books which are supposedly written for some ealier PHP version don't even cover best practices of that time. They're usually many years behind and teach you the techniques of the late 90s. I fear this book is no exception. When I go through the preview, I see table layouts, bgcolor and ereg_replace(). This is not 2009, this is 1999. That doesn't necessarily mean the whole book is garbage. If they at least explain the basics properly, maybe it's worth the effort of unlearning all the old stuff at the end. But if they don't even get that right, then I'd rather look for a more up-to-date resource.