Jump to content

Jacques1

Members
  • Posts

    4,207
  • Joined

  • Last visited

  • Days Won

    209

Posts posted by Jacques1

  1. I think “all fail” is a pretty accurate description of the code. :happy-04:

     

    redarrow: Take a deep breath, try to concentrate for a few minutes and then do this step by step. You need to

    • Create a prepared statement with $conn->prepare()
    • Bind values to that prepared statement
    • Execute the prepared statement
    • Like 1
  2. The problem of Magic Quotes is that they're applied to the input rather than the output, and they're a dumb one-size-fits-all strategy. Both is also true for your approach, even when it's not quite as wrong (because you at least keep the original values).

     

    I understand that you're limited to bad options. The question is whether you've actually picked the least bad option. Your approach is simultaneously confusing and simplistic, so I expect to see a lot of the problems people had in the early 2000s with Magic Quotes: running around with escaped values when you actually need the original ones, running around with the default strategy when you actually need a different one.

     

    If the templates are in fact maintained by (competent) programmers, you should consider replacing the primitive htmlspecialchars()-everything approach with a set of different escape functions for the different contexts. You can catch obvious mistakes (i. e. output values that haven't been escaped at all) with a tainting analysis during development.

     

    And regardless of the approach, you should use Content Security Policy as a second line of defense against XSS -- unless the pages are cluttered with inline JavaScript/CSS or there's yet another crazy policy.

  3. You asked whether your approach is inferior, I gave you an honest answer. If you're forced to make bad decisions due to crazy policies, that's up to you.

     

    The inner workings of Twig escaping are explained in the documentation, the source code is on GitHub. There is no single PHP function you can replace it with, because escaping depends on the context (simple HTML, URLs within HTML, inline CSS, inline JavaScript, ...). When you use htmlspecialchars(), the only safe context is simple HTML. Outside of that, you're wide open to XSS attacks.

     

     

     

    [...] in the rare case they need something intentionally unescaped [...]

     

    What? Using unescaped values is the standard case. The only time you use escaped values is when you output them.

  4. Is this still inferior to twig's escaping?

     

    Yes, because it's cumbersome, confusing and fragile.

     

    Twig automatically escapes the variables when the output is rendered, which is logical (escaping is an output feature) and almost completely eliminates the need for human intervention. Most of the time, the template author doesn't even have to think about XSS vulnerabilities. You, on the other hand, escape the variables as they enter the template, so the author has to constantly decide if they use the raw value or the escaped value. If they get it wrong just once, chances are you'll end up with a nasty bug or a security vulnerability.

     

    And then again: Twig is a lot more than auto-escaping.

  5. PHP and webservers have built-in error handling features. PHP can log error messages and emit a 500 status code for fatal errors. This status code can then be caught by the webserver to display an error page (preferrably a basic static HTML page). See this overview.

     

    If you want a fancier mechanism, you have to implement it yourself or use whatever your framework offers. Be aware that error handling in PHP is quite tricky. There are both exceptions and classical errors, a lot of errors cannot be caught with a simple error handler, and some errors can happen before the application even runs. The only reliably approach I'm aware of is to auto-prepend a script which registers a shutdown handler (as described in the above overview).

  6. Pro tip: Instead of trying to build your own error mechanism, just enable exceptions.

    <?php
    
    $mysqliDriver = new mysqli_driver();
    $mysqliDriver->report_mode = MYSQLI_REPORT_ERROR | MYSQLI_REPORT_STRICT;
    
    $databaseConnection = new mysqli(...);
    

    This will automatically trigger an exception whenever something fails. No need to check return values, no need to pull error codes out of attributes, no need to assemble your own messages.

  7. Long story short: The online course you got your code from sucks and isn't worth your money. They're actively teaching you wrong and dangerous practices which you will have to unlearn if you ever want to have a real website.

     

    Personally, I'm very sceptical about those paint-by-numbers tutorials where they walk you through a (pseudo) project. The quality is usually piss-poor, and even if they get it right, what do you really learn? In my experience, not much. What I recommend is to learn the basics (SQL, PDO, sessions etc.) and then write your own applications, preferrably ones you actually want to use.

  8. Just because IPBoard exposes a user's db ID doesn't mean every other PHP site in existence does.

     

    You couldn't hide the IDs even if you wanted to. And until now, you haven't even managed to come up with a reason for why you want to hide them.

     

     

     

    My problem (which I defined in my first post) was just that I wasn't sure if it was secure/acceptable to store a piece of data in a session variable to track whether or not a user was logged in, or if I should find some other method.

     

    You haven't defined anything. As you already said, you're just throwing random ideas around. If you want an actual security assessment, we need hard facts.

     

    But if you don't think your problems are important, I'm fine with that. Sleep well.

  9. We can use htmlspecialchars() for escaping and can skip using twig for escaping.

     

    Yeah, because escaping every friggin' variable by hand is so much easier than letting the application do it. And of course programmers never screw up – the 10,000 XSS vulnerabilities in the CVE database must be fake.

     

    PHP programmers really are strange. I've offered you a solution which is simultaneously better for you, better for your users and better for everybody else. But you rather keep programming like it was 1995.

  10. It's been three months since our discussion, and you still haven't made up your mind?

     

    Talking about vague rumors isn't very helpful. If you want a performance discussion, there should be a) a real problem (i. e. you're working on a performance-critical application which is already highly optimized) and b) hard facts (benchmarks, results of profiling etc.). It doesn't make sense to worry about a few nanoseconds if this is your personal homepage running on a cheap shared hosts with 10 visitors per month.

     

    As the Stackoverflow thread already explained, Twig templates are compiled to PHP code which is then cached. So there's effectively no overhead.

  11. Is this a one-off project which will only be maintained by you and then thrown away? In that case, you might get away with this hack. But if there's even a remote chance that somebody else may have to work with your code, I strongly recommend against it. Code which arbitrarily throws exceptions and crashes the entire application if not wrapped in plenty of try statements is a problem.

     

    Exceptions are a brutal error mechanism. As you probably know, the default behavior is to immediately halt the script and emit an Internal Server Error, which makes perfekt sense if there is in fact a “hard” error. But you're dealing with “soft” errors, so you need those tricks where you throw an exception only to immediately catch it.

     

    Like I said, this may be “good enough” for a quick hack. Otherwise I suggest you invest some time into a proper error handling mechanism which takes the appropriate actions and emits the right HTTP code (for automated clients, this actually matters).

  12. I see dozens of reasons why this doesn't work. You should read the replies.

     

    This line:

    if (!isset($_SESSION[$uid])) {
    

    uses the content of a variable named “uid” as the index. But there is no such variable in your code. I guess you meant the literal string “user_id”.

     

    Read the replies from your last thread and start programming more systematically. Right now, it seems like you just randomly shuffle fragments around.

  13. Instead of implementing your own ad-hoc template system, how about an actual template engine like Twig? You can then create a macro which has different output depending on a template variable for the mode:

    {% macro product(id, text, plaintext) %}{% if plaintext %}{{ text }}{% else %}<a href="#">{{ text }}</a>{% endif %}{% endmacro %}
    

    Usage:

    The {{ entities.product(123, "Widget", plaintext) }} is a new product ...
    
    • Like 1
  14. That wasn't the question.

     

    flock() is blocking by default: If the file is already locked, then the process is suspended, added to a list and automatically woken up when the file is ready. This means there is no need for any waiting logic. The alternative is non-blocking behavior where the function immediately returns in any case. This implies you do have to write your own waiting logic, and if you get this wrong, a process may lose against other processes over and over again.

     

    Since you appearently don't know what you want, I strongly recommend the default blocking behavior. So get rid of the while loop.

  15. *sigh* Can you be a bit more specific than “doesn't work”? Have you printed the variable content to see if it's even correct?

     

    According to the HTTP standard, the filename must be quoted. As in:

    Content-Disposition: attachment; filename="foo"
    

    Firefox seems to happily accept unquoted filenames, but it will stop after the first space character.

×
×
  • 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.