Loading ...
Sorry, an error occurred while loading the content.
 

Re: "Not reading undo file, owner differs" if the edited file belongs to another user

Expand Messages
  • James McCoy
    ... None of the above disagrees with what I said. You also don t show whether the undo files after steps 4 and 5 are the same inode. It may not be a matter
    Message 1 of 11 , Sep 7, 2013
      On Sat, Sep 07, 2013 at 05:23:13AM -0700, cac2s wrote:
      > I've tried this experiment:
      >
      > 1. $ sudo vim /var/www/htdocs/robots.txt
      >
      > robots.txt is owned by www-data:www-data
      >
      > 2. made some changes in the robots.txt and ":wq"
      >
      > 3. $ sudo ls -la /root/.vim/undofiles/%var%www%htdocs%robots.txt
      > -rw-r----- 1 root www-data 2234 Sep 7 14:43 /root/.vim/undofiles/%var%www%htdocs%robots.txt
      >
      > as you can see the undo file of robots.txt owned by root:www-data now
      >
      > 4. sudo chown cac2s.cac2s /root/.vim/undofiles/%var%www%htdocs%robots.txt
      >
      > "cac2s.cac2s" is chosen only as an example
      >
      > $ sudo ls -la /root/.vim/undofiles/%var%www%htdocs%robots.txt
      > -rw-r----- 1 cac2s cac2s 2234 Sep 7 14:43 /root/.vim/undofiles/%var%www%htdocs%robots.txt
      >
      > 5. made some changes in the robots.txt and ":wq"
      >
      > 6. $ sudo ls -la /root/.vim/undofiles/%var%www%htdocs%robots.txt
      > -rw-r----- 1 root www-data 2234 Sep 7 14:43 /root/.vim/undofiles/%var%www%htdocs%robots.txt
      >
      > the undo file of robots.txt owned by root:www-data again! not by cac2s:cac2s as it was set in step 4.
      >
      >
      > this experiment suggests that vim himself sets the owner of undo file,
      > using the fact that it is running as root.

      None of the above disagrees with what I said. You also don't show
      whether the undo files after steps 4 and 5 are the same inode. It may
      not be a matter of vim changing the ownership of the existing undo file,
      but creating a new file.

      > so maybe correctly set the owner of undo file the same as that of the
      > file being edited, knowing that vim running as root, which guarantees
      > the continued use of the file changes for an undo/redo actions?

      Which only works for root, as I stated before. Non-root users can't set
      arbitrary users as the owner for their files, so you still have a
      situation where it's not possible to make the owner of the undo file
      match that of the file being edited.

      Cheers,
      --
      James
      GPG Key: 4096R/331BA3DB 2011-12-05 James McCoy <jamessan@...>
    • cac2s
      ... I apologize if I gave you a reason to think so. I was just trying to point out is not quite correct actions by vim which running with root privileges. ...
      Message 2 of 11 , Sep 7, 2013
        > None of the above disagrees with what I said.

        I apologize if I gave you a reason to think so. I was just trying to point out is not quite correct actions by vim which running with root privileges.


        > You also don't show whether the undo files after steps 4 and 5
        >
        > are the same inode. It may not be a matter of vim changing the
        >
        > ownership of the existing undo file, but creating a new file.

        $ sudo ls -lai /root/.vim/undofiles/%var%www%vesperaudio.com%robots.txt
        391067 -rw-r----- 1 root www-data 519 Sep 7 15:43 /root/.vim/undofiles/%var%www%vesperaudio.com%robots.txt

        $ sudo chown cac2s.cac2s /root/.vim/undofiles/%var%www%vesperaudio.com%robots.txt

        $ sudo vim /var/www/vesperaudio.com/robots.txt

        made some changes an ":wq"

        $ sudo ls -lai /root/.vim/undofiles/%var%www%vesperaudio.com%robots.txt
        391067 -rw-r----- 1 root www-data 519 Sep 7 15:51 /root/.vim/undofiles/%var%www%vesperaudio.com%robots.txt


        the inode of the undo file is not changed


        > > so maybe correctly set the owner of undo file the same as that of the
        >
        > > file being edited, knowing that vim running as root, which guarantees
        >
        > > the continued use of the file changes for an undo/redo actions?
        >
        >
        >
        > Which only works for root, as I stated before. Non-root users can't set
        >
        > arbitrary users as the owner for their files, so you still have a
        >
        > situation where it's not possible to make the owner of the undo file
        >
        > match that of the file being edited.


        that's why I said about the need to verify that vim running as root:

        > ... knowing that vim running as root...

        --
        --
        You received this message from the "vim_dev" maillist.
        Do not top-post! Type your reply below the text you are replying to.
        For more information, visit http://www.vim.org/maillist.php

        ---
        You received this message because you are subscribed to the Google Groups "vim_dev" group.
        To unsubscribe from this group and stop receiving emails from it, send an email to vim_dev+unsubscribe@....
        For more options, visit https://groups.google.com/groups/opt_out.
      • Bram Moolenaar
        ... So, do you want some arbitrary user be able to undo the changes that root has made? Those are root s changes, in my opinion only root should be able to do
        Message 3 of 11 , Sep 7, 2013
          cac2s wrote:

          > I've tried this experiment:
          >
          > 1. $ sudo vim /var/www/htdocs/robots.txt
          >
          > robots.txt is owned by www-data:www-data
          >
          > 2. made some changes in the robots.txt and ":wq"
          >
          > 3. $ sudo ls -la /root/.vim/undofiles/%var%www%htdocs%robots.txt
          > -rw-r----- 1 root www-data 2234 Sep 7 14:43 /root/.vim/undofiles/%var%www%htdocs%robots.txt
          >
          > as you can see the undo file of robots.txt owned by root:www-data now
          >
          > 4. sudo chown cac2s.cac2s /root/.vim/undofiles/%var%www%htdocs%robots.txt
          >
          > "cac2s.cac2s" is chosen only as an example
          >
          > $ sudo ls -la /root/.vim/undofiles/%var%www%htdocs%robots.txt
          > -rw-r----- 1 cac2s cac2s 2234 Sep 7 14:43 /root/.vim/undofiles/%var%www%htdocs%robots.txt
          >
          > 5. made some changes in the robots.txt and ":wq"
          >
          > 6. $ sudo ls -la /root/.vim/undofiles/%var%www%htdocs%robots.txt
          > -rw-r----- 1 root www-data 2234 Sep 7 14:43 /root/.vim/undofiles/%var%www%htdocs%robots.txt
          >
          > the undo file of robots.txt owned by root:www-data again! not by cac2s:cac2s as it was set in step 4.
          >
          >
          > this experiment suggests that vim himself sets the owner of undo file, using the fact that it is running as root.
          >
          > so maybe correctly set the owner of undo file the same as that of the file being edited, knowing that vim running as root, which guarantees the continued use of the file changes for an undo/redo actions?

          So, do you want some arbitrary user be able to undo the changes that
          root has made? Those are root's changes, in my opinion only root should
          be able to do anything with these changes. Also, we probably don't want
          any other user than root being able to see what changes were made, thus
          the owner of the undo file must be set to the user who made the changes.

          In case the same user edits the file again, then it does make sense to
          use the undo file. So perhaps we can allow reading the undo file if
          it's owned by the current user, even when this differs from the owner of
          the file?

          Try this patch:

          *** ../vim-7.4.022/src/undo.c 2013-06-10 20:13:37.000000000 +0200
          --- src/undo.c 2013-09-07 15:45:56.000000000 +0200
          ***************
          *** 1604,1613 ****

          #ifdef UNIX
          /* For safety we only read an undo file if the owner is equal to the
          ! * owner of the text file. */
          if (mch_stat((char *)orig_name, &st_orig) >= 0
          && mch_stat((char *)file_name, &st_undo) >= 0
          ! && st_orig.st_uid != st_undo.st_uid)
          {
          if (p_verbose > 0)
          {
          --- 1604,1614 ----

          #ifdef UNIX
          /* For safety we only read an undo file if the owner is equal to the
          ! * owner of the text file or equal to the current user. */
          if (mch_stat((char *)orig_name, &st_orig) >= 0
          && mch_stat((char *)file_name, &st_undo) >= 0
          ! && st_orig.st_uid != st_undo.st_uid
          ! && st_undo.st_uid != getuid())
          {
          if (p_verbose > 0)
          {

          --
          hundred-and-one symptoms of being an internet addict:
          181. You make up words that go with the "happy tune" your modem makes
          while dialing your ISP.

          /// Bram Moolenaar -- Bram@... -- http://www.Moolenaar.net \\\
          /// sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\
          \\\ an exciting new programming language -- http://www.Zimbu.org ///
          \\\ help me help AIDS victims -- http://ICCF-Holland.org ///

          --
          --
          You received this message from the "vim_dev" maillist.
          Do not top-post! Type your reply below the text you are replying to.
          For more information, visit http://www.vim.org/maillist.php

          ---
          You received this message because you are subscribed to the Google Groups "vim_dev" group.
          To unsubscribe from this group and stop receiving emails from it, send an email to vim_dev+unsubscribe@....
          For more options, visit https://groups.google.com/groups/opt_out.
        • cac2s
          ... I think in case the undo file is in same directory as the file being edited, checking who is the owner of the undo file should be done, to prevent security
          Message 4 of 11 , Sep 7, 2013
            > So, do you want some arbitrary user be able to undo the changes that
            >
            > root has made? Those are root's changes, in my opinion only root should
            >
            > be able to do anything with these changes. Also, we probably don't want
            >
            > any other user than root being able to see what changes were made, thus
            >
            > the owner of the undo file must be set to the user who made the changes.
            >
            >
            >
            > In case the same user edits the file again, then it does make sense to
            >
            > use the undo file. So perhaps we can allow reading the undo file if
            >
            > it's owned by the current user, even when this differs from the owner of
            >
            > the file?

            I think in case the undo file is in same directory as the file being edited, checking who is the owner of the undo file should be done, to prevent security problems.

            but if all undo files are in the root's home folder, then this check may be omitted since root only has access to the /root

            as an option, you can add a param which will disable the comparing of ownership of the undo files, if you specified the directory for them (set undodir=...)

            how do you like that kind of logic?

            --
            --
            You received this message from the "vim_dev" maillist.
            Do not top-post! Type your reply below the text you are replying to.
            For more information, visit http://www.vim.org/maillist.php

            ---
            You received this message because you are subscribed to the Google Groups "vim_dev" group.
            To unsubscribe from this group and stop receiving emails from it, send an email to vim_dev+unsubscribe@....
            For more options, visit https://groups.google.com/groups/opt_out.
          • Bram Moolenaar
            ... I do not see how the directory of the undo file matters. Unless we also check the ownership of that directory. But that can get complicated. I also don t
            Message 5 of 11 , Sep 7, 2013
              cac2s wrote:

              > > So, do you want some arbitrary user be able to undo the changes that
              > > root has made? Those are root's changes, in my opinion only root should
              > > be able to do anything with these changes. Also, we probably don't want
              > > any other user than root being able to see what changes were made, thus
              > > the owner of the undo file must be set to the user who made the changes.
              > >
              > > In case the same user edits the file again, then it does make sense to
              > > use the undo file. So perhaps we can allow reading the undo file if
              > > it's owned by the current user, even when this differs from the owner of
              > > the file?
              >
              > I think in case the undo file is in same directory as the file being
              > edited, checking who is the owner of the undo file should be done, to
              > prevent security problems.
              >
              > but if all undo files are in the root's home folder, then this check
              > may be omitted since root only has access to the /root
              >
              > as an option, you can add a param which will disable the comparing of
              > ownership of the undo files, if you specified the directory for them
              > (set undodir=...)
              >
              > how do you like that kind of logic?

              I do not see how the directory of the undo file matters. Unless we also
              check the ownership of that directory. But that can get complicated.

              I also don't like making a special case for root, except when it's to
              stay on the safe side. E.g., root changing someone else's may cause the
              original owner to lose undo, since the undo file ownership changed.
              That's OK, because the owner should not be able to use root's changes.
              But root should be able to undo his own changes, which is what my patch
              intends to accomplish.

              --
              hundred-and-one symptoms of being an internet addict:
              182. You may not know what is happening in the world, but you know
              every bit of net-gossip there is.

              /// Bram Moolenaar -- Bram@... -- http://www.Moolenaar.net \\\
              /// sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\
              \\\ an exciting new programming language -- http://www.Zimbu.org ///
              \\\ help me help AIDS victims -- http://ICCF-Holland.org ///

              --
              --
              You received this message from the "vim_dev" maillist.
              Do not top-post! Type your reply below the text you are replying to.
              For more information, visit http://www.vim.org/maillist.php

              ---
              You received this message because you are subscribed to the Google Groups "vim_dev" group.
              To unsubscribe from this group and stop receiving emails from it, send an email to vim_dev+unsubscribe@....
              For more options, visit https://groups.google.com/groups/opt_out.
            • cac2s
              ... I rebuilt vim 7.3.547.7 (debian wheezy) with this patch. Now the undo files are opened just as they should. Thank you! -- -- You received this message from
              Message 6 of 11 , Sep 8, 2013
                > Try this patch:
                >
                > *** ../vim-7.4.022/src/undo.c 2013-06-10 20:13:37.000000000 +0200
                > --- src/undo.c 2013-09-07 15:45:56.000000000 +0200
                > ***************
                > *** 1604,1613 ****
                >
                > #ifdef UNIX
                > /* For safety we only read an undo file if the owner is equal to the
                > ! * owner of the text file. */
                > if (mch_stat((char *)orig_name, &st_orig) >= 0
                > && mch_stat((char *)file_name, &st_undo) >= 0
                > ! && st_orig.st_uid != st_undo.st_uid)
                > {
                > if (p_verbose > 0)
                > {
                > --- 1604,1614 ----
                >
                > #ifdef UNIX
                > /* For safety we only read an undo file if the owner is equal to the
                > ! * owner of the text file or equal to the current user. */
                > if (mch_stat((char *)orig_name, &st_orig) >= 0
                > && mch_stat((char *)file_name, &st_undo) >= 0
                > ! && st_orig.st_uid != st_undo.st_uid
                > ! && st_undo.st_uid != getuid())
                > {
                > if (p_verbose > 0)
                > {


                I rebuilt vim 7.3.547.7 (debian wheezy) with this patch.

                Now the undo files are opened just as they should. Thank you!

                --
                --
                You received this message from the "vim_dev" maillist.
                Do not top-post! Type your reply below the text you are replying to.
                For more information, visit http://www.vim.org/maillist.php

                ---
                You received this message because you are subscribed to the Google Groups "vim_dev" group.
                To unsubscribe from this group and stop receiving emails from it, send an email to vim_dev+unsubscribe@....
                For more options, visit https://groups.google.com/groups/opt_out.
              Your message has been successfully submitted and would be delivered to recipients shortly.