bugGNU nano - Bugs: bug #29732, nano - failure to chmod() results...

 
 

bug #29732: nano - failure to chmod() results in failure to save

Submitted by:  Brian Szymanski <skibrianski>
Submitted on:  Sat 01 May 2010 03:50:37 PM UTC  
 
Category: NoneSeverity: 3 - Normal
Item Group: NoneStatus: Fixed
Privacy: PublicAssigned to: Chris Allegretta <astyanax>
Open/Closed: ClosedRelease: None

Add a New Comment (Rich MarkupRich Markup):
   

You are not logged in

Please log in, so followups can be emailed to you.

 

(Jump to the original submission Jump to the original submission)

Thu 24 Jun 2010 02:59:32 PM UTC, comment #7:

Alright with r4509 I made some additional compatilbility check for allow_insecure_backup.

"I don't think that allow_insecure_backup is the right way to go... The real problem in this case is that failure to create the backup file causes a failure to save the main file. Instead I think we should just warn: "creating backup file '$file~' failed" and proceed to try to save the file in-place? Or at least, that should be the default behavior..."

I don't know that that would be in any way easy to accommodate, because in the scenario you set up with the ACLs, several different system calls will fail during the save of the backup file: unlink(), chmod(), and utime(). We could prompt on the first failure and then set the flag to allow subsequent failures, but that's a larger scale change to string translations, so that would need to be a 2.3 series feature.

The reason we're so pedantic about saving the backup file is that an error with saving the backup file implies that something is out of the ordinary with the system, and means trying to save the actual file is at higher risk to fail in a strange or unanticipated way. Since we have had data loss bugs in the past, I'm unwilling to be lax about what effectively are setups where things like setting the file permissions and modification time on a file which is otherwise editable are failing. I hope this makes sense.

Chris Allegretta <astyanax>
Project AdministratorIn charge of this item.
Mon 21 Jun 2010 11:43:19 PM UTC, comment #6:

Hmm... allow_insecure_backup with r4508 does fix the problem I was seeing with crontab -e, but it DOES NOT fix the problem editing files on ntfs/fat32 partitions or files with extended acls set. I think we need some more work to account for those cases. Assuming you run linux you can reproduce by mounting your fs with -o acl and running the test I showed using setfacl below.

I don't think that allow_insecure_backup is the right way to go... The real problem in this case is that failure to create the backup file causes a failure to save the main file. Instead I think we should just warn: "creating backup file '$file~' failed" and proceed to try to save the file in-place? Or at least, that should be the default behavior...

What do you think?

Brian Szymanski <skibrianski>
Mon 21 Jun 2010 03:14:23 AM UTC, comment #5:

Alright in r4508 I added a new rcfile option "allow_insecure_backup" to override nano bailing when it hits the fchown or chmod errors. I think the option is sufficiently worded to keep people away from setting it accidentally. Let me know if you see any issues with this in SVN, and I'll be making a test release tarball shortly.

Chris Allegretta <astyanax>
Project AdministratorIn charge of this item.
Sun 23 May 2010 04:24:34 AM UTC, comment #4:

Oh wait this is not a dupe, this is about chmod(); updating title.

I certainly have no beef with chown(), but this chmod() issue concerns me. It's going to require more thought because there are security implications of allowing through writing a backup file that one cannot chmod to the original file's permissions, regardless of what odd case has been set up. Perhaps we could add a dialog here but it's concerning. I'll think about it more this weekend.

Chris Allegretta <astyanax>
Project AdministratorIn charge of this item.
Sun 23 May 2010 03:22:08 AM UTC, comment #3:

Closing as duplicate of 29514, we'll see if I can figure out how to add you to that bug's contact list.

Chris Allegretta <astyanax>
Project AdministratorIn charge of this item.
Tue 18 May 2010 03:11:49 PM UTC, comment #2:

Actually this only happens with "set backup" enabled, and it happens more commonly than I thought. Something as simple as crontab -e on ubuntu 10.04 LTS with "set backup" in .nanorc will trigger the bug:

Error writing backup file /tmp/crontab.SJV8Wk/crontab~: Operation not permitted

Clearly this is not the desired behavior in the case of crontab. Failure to write the backup file should result in a warning at most, and above all else should not abort saving the changes made!

I see a few options:
1 - Proceed to try to overwrite the file in place
2 - Prompt the user like so:
"Attempt to save backup failed: Overwrite $FILE without backup? [Y/n]"

I would imagine #1 would be the default, #2 could be enabled with 'set paranoid-backup' or something like that in .nanorc.

Thoughts? Thanks!

Brian Szymanski <skibrianski>
Sat 01 May 2010 04:03:39 PM UTC, comment #1:

oops, i titled the bug wrong. it should say failure to chmod results in failure to save. probably should be merged with the chown() bug as they are closely related, though.

Cheers,
Brian Szymanski

Brian Szymanski <skibrianski>
Sat 01 May 2010 03:50:37 PM UTC, original submission:

Similar to the problem with chown()'ing documented in #29514 (https://savannah.gnu.org/bugs/?29514) , there is also a problem with chmod'ing.

Upon trying to edit a file on a fat32 file system with nano 2.2.2, I get this error:
Error writing backup file /media/common/wifi.txt~: Operation not permitted, and the save aborts (the backup and "real" files on disk remain unchanged).

We can rule out a simple permissions problem:
ski@ganiodayo:~$ ls -l /media/common/wifi.txt
-rwxrwx--- 1 root plugdev 2407 2008-10-23 01:16 /media/common/wifi.txt
ski@ganiodayo:~$ id
uid=1000(ski) gid=1000(ski) groups=4(adm),20(dialout),24(cdrom),46(plugdev),104(fuse),106(lpadmin),121(admin),122(sambashare),1000(ski)
ski@ganiodayo:~$ echo foo >/media/common/wifi.txt
#gives no error

An strace provides this snippet:
open("/media/common/wifi.txt~", O_WRONLY|O_CREAT|O_TRUNC, 0666) = 4
chmod("/media/common/wifi.txt~", 0100770) = -1 EPERM (Operation not permitted)

If chmod fails (as with chown in #29514), we should warn but proceed anyway - because it is entirely possible that we can write to, but can't chmod, the file. In addition to fat32, we can reproduce this bug using file access control lists (getfacl/setfacl) like so:

ski@ganiodayo:~$ sudo touch /tmp/foo /tmp/foo~
ski@ganiodayo:~$ sudo chown root:root /tmp/foo /tmp/foo~
ski@ganiodayo:~$ sudo setfacl -m u:ski:rw /tmp/foo /tmp/foo~
ski@ganiodayo:~$ cat /tmp/foo
ski@ganiodayo:~$ echo 1 >/tmp/foo
ski@ganiodayo:~$ cat /tmp/foo
1
ski@ganiodayo:~$ nano /tmp/foo #try to write "2" and exit, get EPERM
ski@ganiodayo:~$ cat /tmp/foo
1

Brian Szymanski <skibrianski>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach File(s):
   
   
Comment:
   

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by bens (Updated the item)
  • -unavailable- added by astyanax (Posted a comment)
  • -unavailable- added by skibrianski (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 9 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Mon 24 Feb 2014 08:09:27 PM UTCbensStatusReady For Test=>Fixed
      Open/ClosedOpen=>Closed
    Mon 21 Jun 2010 03:14:23 AM UTCastyanaxStatusIn Progress=>Ready For Test
    Sun 23 May 2010 04:24:34 AM UTCastyanaxStatusDuplicate=>In Progress
      Assigned toNone=>astyanax
      Open/ClosedClosed=>Open
      Summarynano - failure to chown() results in failure to save=>nano - failure to chmod() results in failure to save
    Sun 23 May 2010 03:22:08 AM UTCastyanaxStatusNone=>Duplicate
      Open/ClosedOpen=>Closed

    Back to the top


    Powered by Savane 3.1-cleanup