Thanks Nikita, that's good to know. I'm still familiarizing myself with the source right now, so I apologize if this is something that commonly gets spread as false information, I honestly was exploring the workings of injection protection in the source after following this discussion, but that's why I asked. My intention was not to spread FUD, I just thought I'd found something that slipped through the cracks.
Cheers, Jordan On Mon, Jul 19, 2021 at 12:24 AM Nikita Popov <nikita....@gmail.com> wrote: > On Sun, Jul 18, 2021 at 4:42 AM Jordan LeDoux <jordan.led...@gmail.com> > wrote: > >> Related to the general topic of injection attacks, I was considering >> submitting a PR to change the default of PDO::ATTR_EMULUATE_PREPARES to >> FALSE, since this mistakenly can lead people to believe that using >> prepared >> statements with PDO and MySQL protects against injection attacks. In fact, >> this is only the case if they create the PDO object with the option >> specified as false. I'm not aware however to reasoning for enabling >> prepare >> emulation by default for MySQL. I would assume it's a performance choice, >> but how long ago was this choice made and is it worth revisiting? Would >> this be something that requires its own RFC? It's a single line change. >> >> Jordan >> > > Please do refrain from spreading this FUD. While there are certain > tradeoffs between choosing emulated or native prepared statements, security > considerations do not factor into it. There's a very narrow window where > emulated prepared statements can lead to incorrect escaping (it involves > picking an exotic non-ASCII-compatible charset, not specifying it in the > connection DSN, and switching to it at runtime), but it's not something you > can hit by accident. > > Regards, > Nikita > > On Sat, Jul 17, 2021 at 9:48 AM Craig Francis <cr...@craigfrancis.co.uk> >> wrote: >> >> > On Sat, 17 Jul 2021 at 4:05 pm, Marco Pivetta <ocram...@gmail.com> >> wrote: >> > >> > > my belief is that this is not a runtime problem, but rather a >> type-level >> > > issue with tainted/untainted input/output. >> > > >> > >> > >> > Thank you for the feedback Marco, >> > >> > As you appreciate, I don’t believe we can get every PHP developer to use >> > Static Analysis. It’s an extra step that developers with less time, >> energy, >> > or care, will not setup and use. >> > >> > Putting something in the base language, means that libraries can just >> use >> > it, and people using the sites/systems of rushed or lazier developers >> will >> > have these checks helping keep their data secure. Data breeches can have >> > life-changing consequences for people, Injection Vulnerabilities are >> one of >> > the biggest causes of them, and since we have the ability for libraries >> to >> > warn all developers about these mistake, we should. >> > >> > At the moment our house can catch on fire and we don’t even have a smoke >> > alarm. This is the smoke alarm. And there are reasons why it’s builders >> and >> > landlords that have to install them, and we don’t rely on the tenants >> going >> > and sorting them out themselves. Because if they don’t, for the best or >> the >> > worse reasons, either way there are severe consequences to everybody. >> > >> > In regards to Taint Checking, it has a significant problem as it >> creates a >> > false sense of security, hence these examples in the RFC: >> > >> > $sql = 'SELECT * FROM users WHERE id = ' . >> $db->real_escape_string($id); // >> > INSECURE >> > >> > $html = "<img src=" . htmlentities($url) . " alt='' />"; // INSECURE >> > >> > $html = "<a href='" . htmlentities($url) . "'>..."; // INSECURE >> > >> > Fortunately Psalm has just implemented the is_literal() concept, so >> those >> > developers who do use Psalm can protect themselves from these issues: >> > >> > https://github.com/vimeo/psalm/releases/tag/4.8.0 >> > >> > >> > >> > In addition to that, a mechanism to un-taint values is missing, >> > > >> > >> > >> > That’s the main flaw with Taint Checking, because it’s not possible to >> mark >> > something as safe without knowing about the context. As in, developers >> use >> > an escaping function (to mark as untainted), think the value is now >> “safe”, >> > and incorrectly use that value in a way that causes a security >> > vulnerability. >> > >> > is_literal() simplifies this problem considerably, by just identifying >> > developer defined strings, and instead using libraries to handle user >> > values. >> > >> > Craig >> > >> >
