7 ms·
We saw this from facebook few years ago. Now with tumblr. Is there something at core of php that makes this inevitable? I ask this as a concerned php dev(and no
by zaidf 16y ago
We saw this from facebook few years ago. Now with tumblr. Is there something at core of php that makes this inevitable? I ask this as a concerned php dev(and not out of snark).
- tjogin 16y agoI think PHP has so many gotchas, so many ways to trip up even seasoned developers, that it's inevitable to miss something.
- zaidf 16y agoI understand that view. But I am talking specifically about something that would just dump the code of the php file to the browser.
- yrashk 16y agothey just had a single typo i?php instead of <?php I guess somebody's using vim
- jarin 16y agoThat's why I always git diff and run tests before committing, I do that all the time :-/
- drdaeman 16y agoAnd this is the reason one should separate production and development environments and NEVER edit any files on production servers (let `git push` or something similiar handle it)
- russss 16y agoAt Last.fm we had a Subversion pre-commit hook which ensured that all .php files began with <?. That would have easily caught this problem, no drama.
- SoftwareMaven 16y agoUnless the file was checked in separately. If they were doing something like updating the db password, so they need to update "current running" and "up-and-coming" code. As a result, they may have made the change in production, with the typo, and the change in SVN, without the typo. I would put my money on something like this being the cause.
- KevBurnsJr 16y ago+1 to russ. PHP syntax checking on precommit would have caught this. Edit: I'm wrong. It does not catch this.
- zaidf 16y agoNot necessarily. Makes sense for syntax check to only check between php tags. Root of the issue is that by default php outputs anything not between php tags to the browser. Perhaps some sort of buffer control at the top most include can disable output to browser until a later time. That's pretty much how templating systems work except they don't disable output afaik thus leaving possibility of this open.
- dexen 16y agoYou could do ob_start(); include '...'; ob_end_clean();, but it turns out that die() (say, in a custom error handler) () causes an implicit buffer flush. So it's not a foolproof solution.
- bluesnowmonkey 16y agoYou can attach a callback to the buffer to prevent that implicit flush. ob_start(function() {});
- dexen 16y agoThanks, I'll investingate and probably adopt it for my code :D
- KevBurnsJr 16y agoOr register_shutdown_function - http://blog.kevburnsjr.com/php-fatal-error-500 http://blog.kevburnsjr.com/php-fatal-error-500
- bluesnowmonkey 16y agoExactly right. There's no reason for a config file (or any of the setup code) to be writing output. A good way to force this is by immediately starting an output buffer with a callback that returns nothing, then calling ob_end_clean() after setup is complete.
- Maxious 16y agoI think this was more of a systemic failure - the file that failed in this case was also relied upon to disable development mode/error output on production servers. I'm pretty sure my production web servers return a HTTP code 500 with no further output when they encounter an error with the php.ini-production configuration file provided in the PHP source distribution. The hitch was of course that the file in question had a "i?php" (someone was using vi and hit i too many times?) instead of "<?php" which lead the php interpreter to output that section of code as if it were HTML. That feature is something PHP could provide an option to disable and only allow explicit echo/print/printf which should be what templating engines use.
- KevBurnsJr 16y agoActually, PHP returns a 200 when it encounters a fatal error. This explains why it was indexed by Google.
- VMG 16y agoPutting passwords in the php file? That's not a seasoned developer in my mind.
- zaidf 16y agoGood point. Minus the keys/passwords the "harm" would be limited.
- Joakal 16y agoIt's needed to connect to databases with authentication. What would you suggest?
- eam 16y agoI believe the way to do it "correctly" would be to put the actual file containing the credentials in a non-public location and just do an include where you need to access it. At least that's how I do it. I could be wrong...
- windsurfer 16y agoIf you're doing an include, wouldn't the passwords still be in a PHP file?
- lsc 16y agoif the php file is printed rather than executed, the include will not be followed. You'd see the "include /path/to/inaccessible/file" but you wouldn't see the passwords within the include. I think. I haven't seriously used php since 2003 or so.
- russss 16y agoYes, but included files still need a <?php tag at the top, or PHP just prints them out.
- nbpoole 16y ago
- Tomek_ 16y agoPlease name at least 5 of them.
- dexen 16y agoTsk, tsk. It's trendy to blame PHP for every problem out there. Also, it's trendy to drop unseasoned developers onto it and expect them to build complex applications that are publicly accessible yet 100% foolproof. Go figure. If you asked me, I couldn't even tell. I've encoundered some in the past, sure, but found (in docs) both rationale and the correct way to use stuff. There was that reference-vs-value matter when passing objects around, in PHP v4.x, but that's fixed by v5.
- agotterer 16y agoShouldn't this have been caught by checking chsnges before committing them or on a staging server.
- Joakal 16y agoIt looks like they included something that when the PHP parser fails, it reveals the php code. Hence the connection protocols with username and password. Try including the sensitive connection protocols from a non-www directory?
- deleted 16y ago[deleted]
- Loic 16y agoJust put: display_errors = Off is your php.ini and you will never see any errors. This is recommended everywhere for a PHP deployment. ; This directive controls whether or not and where PHP will output errors, ; notices and warnings too. Error output is very useful during development, but ; it could be very dangerous in production environments. Depending on the code ; which is triggering the error, sensitive information could potentially leak ; out of your application such as database usernames and passwords or worse. ; It's recommended that errors be logged on production servers rather than ; having the errors sent to STDOUT. ; Possible Values: ; Off = Do not display any errors ; stderr = Display errors to STDERR (affects only CGI/CLI binaries!) ; On or stdout = Display errors to STDOUT ; Default Value: On ; Development Value: On ; Production Value: Off ; http://php.net/display-errors display_errors = Off
- russss 16y agoI guarantee you that Tumblr has display_errors = Off. This wasn't a display_errors related fuckup, it was a missed opening PHP tag.
- nbpoole 16y agoWell, based on the code that was dumped, it looks like they're using their own error handler which was ignoring display_errors (search for "Use of undefined constant" in the dump).
- CWIZO 16y agoThat custom error handler was never set, because the code you see above never even executed. The reason was, like said many times already, a miss-typed opening PHP tag, which tells PHP where PHP code is. If that tag is missing or miss-typed the code is just returned to the browser like HTML. The point is that no server configuration can save you from an error like this.
- nbpoole 16y ago
- wladimir 16y agoYes. With PHP, you generally put code in the web root. This is a terrible idea, because if your PHP parser becomes non-functional for some reason (or is accidentally disabled) your source code is exposed to the world. With other web frameworks it is generally encouraged to put source code and static content in different directories. With the code completely outside the web root. This is much safer: never put code (or passwords) in your web templates. It is possible to do this with PHP, but in practice almost no one does that. PHP, in my experience, has a lot of these insecure-by-default issues.
- sewerhorse 16y agono. this was a config file with a broken <?php-tag. it'll be treated as a plain text file when included/required.
- russss 16y agoThis bug is nothing to do with putting code in the web root. If you miss the opening <?php tag out in any file, regardless of whether it's in the web root, it will get printed straight out to the browser.
- brown9-2 16y agoThis sounds like "insecure by default" to me - you have to have the opening stanza correct in order for the file contents to be treated as code. Perhaps a more secure idea would be to require an opening stanza for the reverse - for content that should simply be printed to standard out? i.e. <?out to make content that should be outputed, not <?php for content that should be interpreted.
- russss 16y agoSure - I'm not defending PHP. This is all the case for backwards compatibility. The easiest solution would be to define a new file extension/MIME type which is "PHP-by-default".
- 16y ago
- troels 16y agoI believe the main reason you're seeing it with php apps more often, is because php is a very common platform.
- megaman821 16y agoYes, its is that the web root contains code, at the very least an index.php but likely much, much more.
- moeffju 16y agoFacebook had one or more servers in their rotation not configured to actually execute PHP, so it was instead delivered as text/plain. This is a different error - the file was executed by PHP, but since they mangled the opening tag (i?php instead of <?php) it was passed through as HTML by the PHP interpreter. Except for the obvious – never edit files on the live server – other ways to protect against this would be to have multiple opening tags (first line of file would just be <?php ?>, then another opening tag on the second line), have your VCS check that certain files begin with <?php, store the config in a non-executable way (in a YAML file, or in the server environment), or using a combination of file() and eval() to always prepend the '<?php'. And people should really install a PHP error handler first thing - before they load anything else - that delivers errors with a HTTP code 5xx they can catch in their caching layer.
- mseebach 16y ago> Is there something at core of php that makes this inevitable? Yeah, the fact that there is no way to separate code and content. In my past days as a PHP dev, I've done this and similar things many, many times (also putting a space at the beginning or end, and having random other stuff fail because something can't set a header anymore). It has always appeared obvious to me that PHP should define a "pure code" file, one in which "<?php" would be illegal syntax, and which would never have its content written to the client. That's how nearly all PHP is written, anyway, and it would eliminate a good deal of stupid errors.