that PHP fix is 13k lines
5k apparently of non-tests
that's all human written, right? I wouldn't want to speak out of turn about sprawling quick fix diffs
Discussion
that PHP fix is 13k lines
5k apparently of non-tests
that's all human written, right? I wouldn't want to speak out of turn about sprawling quick fix diffs
@davidgerard On an extremely cursory look at the patch diff, the code is of course awful because it's in C, but it looks like human-writen awful C. No obvious AI "tells" have jumped out at me. The tests even appear to test the fixes rather than just print "yeah, everything's just fine, ship it".
It looks like several people have in fact put in quite a heroic effort to fix this, for which they should get a commendation, but it's a shame that they have to waste their talents on this unmaintainable mudball.
@pndc @davidgerard What I was told is that the fix process for this vuln (actually pair of vulns) was a process that lasted about a month and crossed multiple organizations
@pndc @davidgerard oh I have plenty of obvious AI ones when I looked through earlier:
- changed test cases to something different for no reason. I talked about this earlier https://chitter.xyz/@JennyFluff/117479752875775371
- out of memory checks on each iteration of a loop instead of growing the thing first, checking it and then coping over the data
random function return type changes for no reason
- several function return types were changed from void to bool for no apparent reason.
- gates on the newly added return type changed function return true or SUCCESS depending on the file (when if you'd even do that return false at a failed gate!)
- sometimes they handle OOM errors with a goto, sometimes with returns. Sometimes goto statements jump to different labels instead of just oom, like oom_function_name
- sometimes they gate OOM error handling with some other condition that makes no sense
This is a dumpster fire of a patch.
@davidgerard It doesn't look LLM generated at first blush, but I'm no longer confident in my ability to tell to be honest. Either way, you could not possibly pay me enough to sign off on +11k -1k over 423 files with most of it being random branches and gotos...
The fix for EOL versions are ~300 lines.
- 7.4: https://github.com/php/php-src/commit/712e60f9edfd03f0960fc4fc3c2c375f42b90aa2
- 8.0: https://github.com/php/php-src/commit/82f8f31b16a035c1b67cc253e79f7cc288f4cc20
- 8.1: https://github.com/php/php-src/commit/1d7dec178a094e6e75b229ce22754045f8c6070a
@corbet i just spent my entire day and missed doing today's post/episode because of it, so i'm not enamoured of PHP
i expect the PHP team is not having the greatest day either
@davidgerard according to the blog post that went with this security release, they've been working on this patch for about 2 MONTHS, so this gigapatch may very well likely be human
@davidgerard after seeing some more detailed analysis on the patch I may need to retract my previous toot...
@techokami wondering which human put a little juicing in their code
@davidgerard I feel like retreating tothe one language not rotted with slop... Motorola 68000 assembly
@davidgerard php team busy at work
@stoori i expect they're not having the greatest day either
@davidgerard Even if this isn't that, at *some* point we're going to see a quick "everyone panic! Keystone tool XYZ is compromised!" 13kloc fix that fixes … something … then deliberately adds three more well-obfuscated backdoors.
@cstross if we haven't seen it yet we haven't noticed it
vibe code is a playground for the bad guys
@davidgerard We've spent more than 75 years trying to keep code and data separate and vibe coding just drunk-drove a bulldozer through a lifetime of hard-learned lessons about best practices.
@cstross @davidgerard Everyone knows the solution to in-band signaling is more in-band signaling with a lid of extra begging and pleading on top.
@davidgerard just a quick 13kloc fix for my personal homepage tools
@davidgerard 🫠 oh no