FS#18746 - [webfs] only primary group checked for read access

Attached to Project: Community Packages
Opened by Matthew (piezoelectric) - Thursday, 18 March 2010, 22:20 GMT
Last edited by Dan Griffiths (Ghost1227) - Monday, 29 March 2010, 21:24 GMT
Task Type Bug Report
Category Upstream Bugs
Status Closed
Assigned To Dan Griffiths (Ghost1227)
Architecture All
Severity Low
Priority Normal
Reported Version
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Votes 1
Private No

Details

Currently, webfsd only checks for read access for the gid of the process for read permissions. The supplementary groups of the uid are ignored. This results in directory listings failing to show links for many entries.

Yes, this is an upstream bug, and even documented in the source, but upstream is not responsive.

I've attached a patch which fixes the issue (and IMHO makes the read permission checking my elegant).
   ls.c.patch (0.6 KiB)
This task depends upon

Closed by  Dan Griffiths (Ghost1227)
Monday, 29 March 2010, 21:24 GMT
Reason for closing:  Implemented
Additional comments about closing:  Thanks to piezoelectric for the patch!
Comment by Matthew (piezoelectric) - Saturday, 20 March 2010, 05:44 GMT
Can this be moved to community? My mistake.
Comment by Matthew (piezoelectric) - Saturday, 20 March 2010, 23:15 GMT
This should still be moved to the correct project, however some new information has come to my attention about the supplementary group ID's of a process. This patch should not regress the current behavior of webfsd, but it won't help in ALL situations.
Comment by Matthew (piezoelectric) - Tuesday, 23 March 2010, 10:57 GMT
Dan,  FS#18797  was marked as a duplicate of this bug, but it's not.  FS#18797  is an actual security issue.

This issue pertains to an unimplemented feature in the source code. I have a patch for all of it (this and  FS#18797 ), however there was one subtlety which I was unsure of: webfsd is clearly written with support for setuid and setgid, however setuid-root does not allow for the -u and -g options; I am not sure if this is intentional or not. Because of this, the original patch I provided does not work if the setuid bit is set.
Comment by Dan Griffiths (Ghost1227) - Tuesday, 23 March 2010, 19:46 GMT
You realize you just told me that this was a duplicate of itself?
Comment by Matthew (piezoelectric) - Tuesday, 23 March 2010, 22:35 GMT
Sorry, I edited my original comment accordingly.
Comment by Dan Griffiths (Ghost1227) - Wednesday, 24 March 2010, 19:42 GMT
Are you planing on updating your patch? What harm would be caused by updating using the current patch?
Comment by Matthew (piezoelectric) - Wednesday, 24 March 2010, 23:38 GMT
I am going to update all of the issues but the (IMO) mishandling of setuid. There's no use applying the current patch. Please give me a night or two as I cannot work on this at work.
Comment by Matthew (piezoelectric) - Sunday, 28 March 2010, 15:12 GMT
Here you go. I tried to test with as many scenarios as I could possibly think of. I've commented on the changes in the diff file, but here they are for your convenience:

1) user/group names my now be set to the system maximum using
sysconf(_SC_LOGIN_NAME_MAX). They were previously hardcoded to 16 chars.

2) supplementary groups are now set according to the user webfs is running as.
previously they were left as the calling user, which could be dangerous
ex: sudo webfsd -u nobody, would leave webfsd with all of root's groups!

3) the supplementary group list is no longer made empty when using -g

4) supplementary groups are now checked for read access when generating
directory listings

5) in ls.c/ls() changed type of uid and gid to uid_t and gid_t

6) in ls.c/ls() fixed a problem where the uid of the file was being compared
to the gid of the user to check for readability

7) added a -G option to ignore/remove all supplementary groups

8) updated man page to reflect -G option

9) when the uid is 0, all files are now displayed as readable.
   webfs.patch (7.9 KiB)
   PKGBUILD (0.9 KiB)
   PKGBUILD (0.9 KiB)
Comment by Matthew (piezoelectric) - Sunday, 28 March 2010, 15:16 GMT
Gah, I'm sorry. I forgot to increase the release number in the PKGBUILD. I don't have permission to delete the incorrect file it seems...

Loading...