9 ms·

Ah the old, break the PoC to make the researcher stop complaining move but don't fix the underlying insanity. Classic.

david_shaw 11y ago
My background's in application security assessments. I've seen this hundreds (or more) times, from developers that should really know better. "Hey, there's SQLi in this input form! Better make sure ' OR 1=1;-- is blacklisted," but don't properly parameterize their queries or sanitize input.
- dsacco 11y ago
  "Hey, they reported cross-site scripting! Let's blacklist angle brackets, that'll do the trick!" In case this is not clear to anyone in 2016, blacklisting known-dangerous characters is not an adequate bug fix. It's a rabbit hole, you will burn hours trying to blacklist every character or character combination that can cause a vulnerability just to have someone own you anyway.
  - TTPrograms 11y ago
    What's current best practice?
    - sarciszewski 11y ago
      The proper fixes for common web application vulnerabilities are as follows: Session Hijacking/Fixation/etc.: Use TLS. SQL Injection: Prepared statements that AREN'T emulated; PHP's defaults are bad here. EDIT: If you're writing in another language, make sure it's not providing string escaping masquerading as prepared statements, but actual prepared statements. (My earlier comment was too broad; some forms of emulated prepared statements might be OK, but PHP's is dangerous.) Cross-Site Scripting: Context-aware escaping (templating libraries) + Security Headers Cross-Site Request Forgery: CSRF tokens Password storage: bcrypt, scrypt, PBKDF2-SHA2, Argon2 Encryption, Digital Signatures, Authenticated Key Exchanges, etc.: Hire an expert, don't do it yourself based on the advice contained within HN comments. File Inclusion / Directory Traversal: Don't write your applications in a dumb way that makes these vulnerabilities possible. But if you must, use something like realpath() with a sanity check based on the expected parent directory (in PHP). XML External Entities: Make sure you disable the entity loader: libxml_disable_entity_loader(true); PHP Object Injection in PHP 5: don't ever pass user input to unserialize(); use json_decode() instead. PHP Object Injection in PHP 7: either disable object loading or whitelist the allowed types; i.e. unserialize($var, false); or unserialize($var, ['DateTime']); These are just some of the common problems I frequently find, of course. There are more basic ways to mess up an application ("not even checking that you're authenticated" being at the top of that list). https://paragonie.com/blog/2015/08/gentle-introduction-application-security https://paragonie.com/blog/2015/08/gentle-introduction-appli... Further reading and resources: * https://securityheaders.io https://securityheaders.io * https://github.com/paragonie/awesome-appsec https://github.com/paragonie/awesome-appsec And if anyone wants their code reviewed: https://paragonie.com/services https://paragonie.com/services
      - derefr 11y ago
        Do prepared statements count as emulated if the DB doesn't support prepared statements, but the DB adapter is doing replacement during the encoding-to-typed-binary-wire-protocol step (i.e. replacement of typed tokens with other typed tokens) rather than by just concatenating strings?
        sarciszewski 11y ago
        By prepared statements, I mean your application actually sends the query string in a separate packet from the data, and thereby gives the data no opportunity to corrupt the query string. You can stop all known attacks with escaping, but then you run into fun corner cases like http://stackoverflow.com/a/12118602/2224584 http://stackoverflow.com/a/12118602/2224584 What PHP does is silently perform string escaping for you instead of doing a prepared statement. This is stupid, but PHP Internals discussions are painful (so changing it is unlikely to happen any time soon) and the userland fix is easy: https://github.com/paragonie/easydb/blob/f90fbca34ac7b7b96f7473c330629c084c115137/src/EasyDB.php#L20 https://github.com/paragonie/easydb/blob/f90fbca34ac7b7b96f7... If you're sending a 1+N packets (for N >= 1) to your RDBMS for each new query, then you're probably using prepared statements.
        derefr 11y ago
        That doesn't really address my question. There are real prepared statements like you're talking about; there's the crap PHP does; and then there's what you get if you use e.g. Erlang's Postgres library, which is that you pass it this: execute("SELECT foo FROM bar WHERE baz = ?", [5]) and it becomes something like this: db_socket ! encode_to_wire_format( {'SELECT', "foo", "bar", [{'baz', 5}]} )) Postrges's prepared statements aren't being used, but the distinction between "tainted" user-generated data and the "trusted" statement is maintained, because the 5 in the above is typed data being sent over the wire in a length-prefixed binary encoding, rather than string data being serialized+escaped into another string. Which is to say, if you (or your users) tried to put a fragment of SQL in place of the 5 above, it'd just get treated as string-typed data, rather than SQL. You don't need packet-level separation to achieve that. But is this approach still bad for "emulating" prepared statements, somehow? I don't see how.
  - deleted 11y ago
    [deleted]

My background's in application security assessments. I've seen this hundreds (or more) times, from developers that should really know better. "Hey, there's SQLi in this input form! Better make sure ' OR 1=1;-- is blacklisted," but don't properly parameterize their queries or sanitize input.

dsacco 11y ago
"Hey, they reported cross-site scripting! Let's blacklist angle brackets, that'll do the trick!" In case this is not clear to anyone in 2016, blacklisting known-dangerous characters is not an adequate bug fix. It's a rabbit hole, you will burn hours trying to blacklist every character or character combination that can cause a vulnerability just to have someone own you anyway.
- TTPrograms 11y ago
  What's current best practice?
  - sarciszewski 11y ago
    The proper fixes for common web application vulnerabilities are as follows: Session Hijacking/Fixation/etc.: Use TLS. SQL Injection: Prepared statements that AREN'T emulated; PHP's defaults are bad here. EDIT: If you're writing in another language, make sure it's not providing string escaping masquerading as prepared statements, but actual prepared statements. (My earlier comment was too broad; some forms of emulated prepared statements might be OK, but PHP's is dangerous.) Cross-Site Scripting: Context-aware escaping (templating libraries) + Security Headers Cross-Site Request Forgery: CSRF tokens Password storage: bcrypt, scrypt, PBKDF2-SHA2, Argon2 Encryption, Digital Signatures, Authenticated Key Exchanges, etc.: Hire an expert, don't do it yourself based on the advice contained within HN comments. File Inclusion / Directory Traversal: Don't write your applications in a dumb way that makes these vulnerabilities possible. But if you must, use something like realpath() with a sanity check based on the expected parent directory (in PHP). XML External Entities: Make sure you disable the entity loader: libxml_disable_entity_loader(true); PHP Object Injection in PHP 5: don't ever pass user input to unserialize(); use json_decode() instead. PHP Object Injection in PHP 7: either disable object loading or whitelist the allowed types; i.e. unserialize($var, false); or unserialize($var, ['DateTime']); These are just some of the common problems I frequently find, of course. There are more basic ways to mess up an application ("not even checking that you're authenticated" being at the top of that list). https://paragonie.com/blog/2015/08/gentle-introduction-application-security https://paragonie.com/blog/2015/08/gentle-introduction-appli... Further reading and resources: * https://securityheaders.io https://securityheaders.io * https://github.com/paragonie/awesome-appsec https://github.com/paragonie/awesome-appsec And if anyone wants their code reviewed: https://paragonie.com/services https://paragonie.com/services
    - derefr 11y ago
      Do prepared statements count as emulated if the DB doesn't support prepared statements, but the DB adapter is doing replacement during the encoding-to-typed-binary-wire-protocol step (i.e. replacement of typed tokens with other typed tokens) rather than by just concatenating strings?
      - sarciszewski 11y ago
        By prepared statements, I mean your application actually sends the query string in a separate packet from the data, and thereby gives the data no opportunity to corrupt the query string. You can stop all known attacks with escaping, but then you run into fun corner cases like http://stackoverflow.com/a/12118602/2224584 http://stackoverflow.com/a/12118602/2224584 What PHP does is silently perform string escaping for you instead of doing a prepared statement. This is stupid, but PHP Internals discussions are painful (so changing it is unlikely to happen any time soon) and the userland fix is easy: https://github.com/paragonie/easydb/blob/f90fbca34ac7b7b96f7473c330629c084c115137/src/EasyDB.php#L20 https://github.com/paragonie/easydb/blob/f90fbca34ac7b7b96f7... If you're sending a 1+N packets (for N >= 1) to your RDBMS for each new query, then you're probably using prepared statements.
        derefr 11y ago
        That doesn't really address my question. There are real prepared statements like you're talking about; there's the crap PHP does; and then there's what you get if you use e.g. Erlang's Postgres library, which is that you pass it this: execute("SELECT foo FROM bar WHERE baz = ?", [5]) and it becomes something like this: db_socket ! encode_to_wire_format( {'SELECT', "foo", "bar", [{'baz', 5}]} )) Postrges's prepared statements aren't being used, but the distinction between "tainted" user-generated data and the "trusted" statement is maintained, because the 5 in the above is typed data being sent over the wire in a length-prefixed binary encoding, rather than string data being serialized+escaped into another string. Which is to say, if you (or your users) tried to put a fragment of SQL in place of the 5 above, it'd just get treated as string-typed data, rather than SQL. You don't need packet-level separation to achieve that. But is this approach still bad for "emulating" prepared statements, somehow? I don't see how.
- deleted 11y ago
  [deleted]