The permissions of the files in the root folder are not correct: these files are executable but are not supposed to be. The files in the includes/ folder are fine.

Patch follows.

CommentFileSizeAuthor
#1 1307094-1-nodeaccess_userreference-fix_permissions.patch902 bytespfrenssen
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

pfrenssen’s picture

pfrenssen
Location Sofia
CreditAttribution: pfrenssen commented
Status: Active » Needs review
FileSize
1307094-1-nodeaccess_userreference-fix_permissions.patch902 bytes
danielb’s picture

danielb CreditAttribution: danielb commented

File permissions?
Huh?
Can you give more information?

danielb’s picture

danielb CreditAttribution: danielb commented
Status: Needs review » Postponed (maintainer needs more info)

I can't find any info that you can keep permissions on a file after transferring it, zipping it, emailing it, etc... ?
Plus when I look in my copy of the module directory on my server; it is already 644.

danielb’s picture

danielb CreditAttribution: danielb commented

$ git apply -v 1307094-1-nodeaccess_userreference-fix_permissions.patch
Checking patch README.txt...
warning: README.txt has type 100644, expected 100755
Checking patch nodeaccess_userreference.info...
warning: nodeaccess_userreference.info has type 100644, expected 100755
Checking patch nodeaccess_userreference.install...
warning: nodeaccess_userreference.install has type 100644, expected 100755
Checking patch nodeaccess_userreference.module...
warning: nodeaccess_userreference.module has type 100644, expected 100755

To me that suggests the file is already 644?

Not that I think it matters. Isn't this really up to the person installing the module? Any docs that say otherwise?

pfrenssen’s picture

pfrenssen
Location Sofia
CreditAttribution: pfrenssen commented

In the git repository the permissions are not correct. If I check out the latest repository and look at the files I see that the files in the root folder are executable.

$ git clone --branch 7.x-3.x http://git.drupal.org/project/nodeaccess_userreference.git
Cloning into nodeaccess_userreference...
remote: Counting objects: 374, done.
remote: Compressing objects: 100% (240/240), done.
remote: Total 374 (delta 234), reused 212 (delta 132)
Receiving objects: 100% (374/374), 76.62 KiB, done.
Resolving deltas: 100% (234/234), done.
$ cd nodeaccess_userreference/
$ ls -l
total 36
drwxr-xr-x  4 pieter pieter   160 2011-10-13 11:35 ./
drwxrwxrwt 18 root   root     460 2011-10-13 11:35 ../
drwxr-xr-x  8 pieter pieter   260 2011-10-13 11:35 .git/
drwxr-xr-x  2 pieter pieter   100 2011-10-13 11:35 includes/
-rwxr-xr-x  1 pieter pieter   274 2011-10-13 11:35 nodeaccess_userreference.info*
-rwxr-xr-x  1 pieter pieter  6340 2011-10-13 11:35 nodeaccess_userreference.install*
-rwxr-xr-x  1 pieter pieter 20350 2011-10-13 11:35 nodeaccess_userreference.module*
-rwxr-xr-x  1 pieter pieter   807 2011-10-13 11:35 README.txt*

I'll have a look in the documentation about the guidelines.

pfrenssen’s picture

pfrenssen
Location Sofia
CreditAttribution: pfrenssen commented

I can't find an explicit mention in the documentation, but it seems like common sense to make files non-executable and directories executable.

I asked on IRC and xjm pointed me to this issue: #1113148: File permissions inside a git repository

pfrenssen’s picture

pfrenssen
Location Sofia
CreditAttribution: pfrenssen commented
Status: Postponed (maintainer needs more info) » Needs review
danielb’s picture

danielb CreditAttribution: danielb commented
Status: Needs review » Closed (won't fix)

In the git repository the permissions are not correct.

I don't see evidence of this. What you've shown is what the files look like on your server/computer.

I also asked on IRC and they said not to worry about this issue.

danielb’s picture

danielb CreditAttribution: danielb commented
Status: Closed (won't fix) » Fixed

Well since it's an easy commit, I'll commit it.
I'm curious if this does actually make a difference on your end?

danielb’s picture

danielb CreditAttribution: danielb commented

I've committed it.

I don't know what good it will do if this is a real issue, since if something caused this in the first place it is bound to happen again. I use windows to develop and commit, etc.. so who knows.

Installing the current(old) version of the module on a unix setup gives me 644 everytime.

pfrenssen’s picture

pfrenssen
Location Sofia
CreditAttribution: pfrenssen commented

Ouch I think you accidentally committed the patch from #1305886: Extend documentation.

It is indeed because I work with linux that I have noticed this issue in the first place. All executable files are highlighted in a very bold colour, so they stood out when I browsed through the module files.

Anyway thanks for the great module and your patience!

danielb’s picture

danielb CreditAttribution: danielb commented
Status: Fixed » Active

lol woops

danielb’s picture

danielb CreditAttribution: danielb commented

I don't think that's what happened, Readme looks the same as it did before :/

I'll try again

danielb’s picture

danielb CreditAttribution: danielb commented
Status: Active » Fixed

I must have done it right because if I apply the patch now it says "nothing to commit".

I think the real answer is that this file permissions thing isn't commit-able.

danielb’s picture

danielb CreditAttribution: danielb commented
Status: Fixed » Closed (won't fix)

http://drupalcode.org/project/nodeaccess_userreference.git/commitdiff/a7...

I'm guessing this is why you thought it was the patch from the other issue. Looks like all that changed is \r\n linebreaks got changed to \n, which must have been an incidental thing that happened from dealing with the other issue - I must have saved it in unix format, which is still a bonus.

It might be that I can't commit your permissions patch because I'm running gitbash in windows, and applying the patch with file permissions might not actually affect the file?

pfrenssen’s picture

pfrenssen
Location Sofia
CreditAttribution: pfrenssen commented

I suppose the actual danger is rather limited. If an attacker has sufficient permissions to inject malicious code in these files there are probably other more important security holes to worry about.

If you ever get your hands on a linux or OSX machine it would be nice if you could commit it after all, I suppose it does not work on Windows because it handles permissions differently than *nix. When I try it here on the latest dev it works without a hitch:

$ git clone --branch 7.x-3.x http://git.drupal.org/project/nodeaccess_userreference.git
Cloning into nodeaccess_userreference...
remote: Counting objects: 377, done.
remote: Compressing objects: 100% (243/243), done.
remote: Total 377 (delta 235), reused 211 (delta 132)
Receiving objects: 100% (377/377), 77.11 KiB, done.
Resolving deltas: 100% (235/235), done.
$ cd nodeaccess_userreference/
$ wget -q http://drupal.org/files/1307094-1-nodeaccess_userreference-fix_permissions.patch
$ git am 1307094-1-nodeaccess_userreference-fix_permissions.patch
Applying: Issue #1307094: Fix file permissions.
$ ls -l
total 40
drwxr-xr-x  4 pieter pieter   180 2011-10-15 12:49 ./
drwxrwxrwt 17 root   root     440 2011-10-15 12:48 ../
-rw-r--r--  1 pieter pieter   902 2011-10-12 12:56 1307094-1-nodeaccess_userreference-fix_permissions.patch
drwxr-xr-x  8 pieter pieter   280 2011-10-15 12:49 .git/
drwxr-xr-x  2 pieter pieter   100 2011-10-15 12:48 includes/
-rw-r--r--  1 pieter pieter   274 2011-10-15 12:49 nodeaccess_userreference.info
-rw-r--r--  1 pieter pieter  6340 2011-10-15 12:49 nodeaccess_userreference.install
-rw-r--r--  1 pieter pieter 20350 2011-10-15 12:49 nodeaccess_userreference.module
-rw-r--r--  1 pieter pieter   776 2011-10-15 12:49 README.txt
danielb’s picture

danielb CreditAttribution: danielb commented
Issue summary: View changes

https://www.drupal.org/node/1988384