None of files in the distribution package should have "executable" bit set.

CommentFileSizeAuthor
#15 executable_php.png76.53 KBgreggles
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

danielb’s picture

Status: Active » Closed (won't fix)

I have received these sorts of reports in the past and nobody has been able to show me any evidence that this is a module developers responsibility. In particular, there is no tool available to me to ensure that files you extract from a repository or from an archive file have a particular permission in your operating system.
IMO this is something another module, or Drupal itself, or the admin, or the web developer would be suitably equipped to handle.

ram4nd’s picture

Status: Closed (won't fix) » Active
danielb’s picture

Status: Active » Postponed (maintainer needs more info)

That is a guide for site owners, I don't see anything in there about a module maintainers responsibility regarding this? The implication I got from reading that page is that Drupal hosted files will automatically have their permissions set properly.
I do all my development and repository interaction from my IDE in Windows. I have never set or unset or changed a file's linux permissions, that is not possible for me to do.

ram4nd’s picture

What part don't you understand? It is purely a security thing. You can run security review module and you can see that your modules fails. "Safe file system permissions (protecting against arbitrary code execution)." I don't know any other way how to explain this to you. But, yes I know that these problems come from windows machines.

ram4nd’s picture

Status: Postponed (maintainer needs more info) » Active
danielb’s picture

I comprehend what you're saying but I disagree about it being something I should fix.

I am not convinced that it is a Node export issue. The page you've linked does not say anywhere that a module maintainer has to change file permissions. I was asking for documentation that instructs a module developer on their responsibilities about this. I haven't been able to find any. I'm not saying I don't have that responsibility, but if I do then I don't know that I do and what is expected of me and what is the best way to go about doing it. What you've shown is a guide for site owners as far as I can tell, and has a section for sysadmins. It talks about servers, and apache, and running the modules - not about writing them.

You can run security review module and you can see that your modules fails. "Safe file system permissions (protecting against arbitrary code execution)."

The security review module shows that files on your own file system, that you are responsible for, need a permission change. If you make that change the problem is solved. The link you've provided seems to suggest that it is a site owners/admins responsibility to ensure these things. That module is designed to help you do that. I suspect your expectations of what maintainers should do may be misplaced. If it was necessary for me to set permissions I think there would be word about it from an official source. If it is a genuine security issue, then the security team would have addressed that. Furthermore can you demonstrate that the security review module is correct about "arbitrary code execution" ? Please do it with this module and show me how to do it, and how it can be used for malicious purposes, and how changing a file permission solves that scenario.

But, yes I know that these problems come from windows machines.

Yes, so even if a patch is applied, or someone fixes it, the next time a file is updated it seems like the fix may regress. It is not wrong for developers to use Windows to code and apply patches. Is the expectation that every change I make should be followed by me transferring the file to a linux machine, go through all the permissions of every file, and do git commits from there? This seems unreasonable and is the main reason I have to resist this unless something says I 100% have to do this. If something says I have to do this, then I would want to have a chat with the person who wrote that and try to find another solution, preferably with automation - as the workflow I've outlined is slow and painful.

The distribution package that you obtain when you checkout this module, or when download the archive files don't come from me. I don't zip them, I don't tar them, I don't even have the final edit of those files (If you open up the package, the .info file has "Information added by Drupal.org packaging script"). All I do is add code to a repo, tag it, and create a page on this website describing the tag. If any of this permissions stuff was enforced by Drupal.org they would surely add something to the packaging script to do this (if it doesn't already do that), and to the .info files to override it is needed.

I'm happy to be proven wrong, but I suspect since you haven't been able to provide the evidence for that - it doesn't exist.

If the evidence I'm looking for isn't presented I will close this issue because I don't believe I can act on it. If you believe this module has a security problem then report it to the Drupal.org Security team and they can advise me on their expectations and I can pose these questions to them.

ram4nd’s picture

If you believe this module has a security problem then report it to the Drupal.org Security team.

I did, will see what they say.

Pere Orga’s picture

@danielb file permissions that are in the git repo end up in the release tarball. In many cases these permissions will be deployed in a site, for example when the module is downloaded via drush on unix-like machines. You are right that there is no guarantee that these file permissions will be kept forever on a website, but IMHO that will happen in the majority of sites.

I've failed to find the official documentation about this, but in my opinion it's clear that this should to be fixed in the module.

Pere Orga’s picture

To fix the permissions issue on windows, have you tried https://www.drupal.org/node/1189520 ?

danielb’s picture

Which files would you like me to run that command on?
(when I put the files on a linux server they're all 0644 to me)

ram4nd’s picture

Files are 644 and directories are 755.

greggles’s picture

This has been "fixed" by the drupal.org packaging script to remove writability from files, which is the most important thing from a security perspective afaik. Please see #116431: Wrong directory permissions.

If anyone thinks that having executable set on these files is a problem as well, let's discuss in that issue.

danielb’s picture

Status: Active » Closed (won't fix)

I suspect a lot of this is superstition. An OS can't "execute" a .php script, it's just a bunch of text. I also think the release packager should be in charge of these decisions just for the sake of consistency.
As much fun at this Borat sketch of an issue has been, if I am to be convinced that something needs to change it will require a knowledgeable person that can level with me about what I've said, particularly regarding practicality.

greggles’s picture

Status: Closed (won't fix) » Active

The packaging script takes care of people using a tarball, but if they clone directly from git it doesn't provide any help.

An OS absolutely can execute a php script, though not in the format of the files in this project. See attached, though I'm sure you know all that already. So, if someone is a sysadmin and looking to make sure their server is secure, it makes sense to be wary of executable php files.

If any of the files or folders were writable by a "g" or "o" then that would present a definite security problem.

Is there a reason you like making the files executable?

I read #6 and agree with many of your points. One thing you said:

the next time a file is updated it seems like the fix may regress.

Did you try it and see? I believe that the correct value will persist and once you fix it you won't have any more problems. It seems like your position is "I shouldn't do this since it will create more work both up front and ongoing and it has little actual benefit." I agree with the idea that the actual benefit is limited, but I'm not sure about how much work it is.

There's also a point that the executable permission is just plain wrong - these files are not intended to be executed as shell commands. Making them executable advertises them for a purpose they are not designed.

greggles’s picture

FileSize
76.53 KB

Forgot to attach the file.

danielb’s picture

I didn't know about that second method. Doesn't that mean, though, that you still have access to the PHP interpreter? So you can already execute arbitrary code?

Did you try it and see?

I believe so, years ago, an issue like this came up, it got fixed and it didn't stick.

Is there a reason you like making the files executable?

I have never made a file executable. That's what worries me, is these things will just happen with no deliberate action on my part.

So back to the suggestion in #9, that actually works. But since nobody reported which files have the wrong permissions, and simply uploading them on a linux server doesn't reveal any permissions issues, so that method isn't going to do me any good. I have a centos VM somewhere so when I have time I can investigate it that way.

ram4nd’s picture

http://cgit.drupalcode.org/node_export/tree/ Please remove execution permissions from files.

danielb’s picture

Here's the commit for reference, because I didn't quote the issue number as typing in the vm is painful
http://cgit.drupalcode.org/node_export/commit/?id=f6cbe74

danielb’s picture

Status: Active » Fixed
greggles’s picture

Thanks, danielb!

And yes, invoking php on the cli shows you have access to the php. Although in a shared hosting environment, the "o" user likely will have that permission.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.