[haiku-development] Re: Information security based small task

  • From: Rohit Yadav <rohityadav89@xxxxxxxxx>
  • To: haiku-development@xxxxxxxxxxxxx
  • Date: Thu, 4 Nov 2010 01:02:22 +0530

Hello again,

This should fix the same issue with ext2fs, using similar approach. I tested
it and it works for me.
Both the patches are attached to the ticket.
Let me know if this issue is still there in any other filesystem, or the
patch needs correction.
(removed a duplicate #include already in Inode.h in ext2)

Best Regards,
Rohit Yadav

On Wed, Nov 3, 2010 at 5:26 PM, Rohit Yadav <rohityadav89@xxxxxxxxx> wrote:

> Hello,
>
> Based on the discussion on http://dev.haiku-os.org/ticket/6750, I tried to
> rework the solution.
> In this implementation of Inode::CheckPermissions, we supply a mask and
> accordingly take action.
> So, this will work for all:
>
> if ((mode & B_STAT_MODE) != 0) {
>     status_t status = CheckPermissions(W_OK, B_STAT_MODE);
>     if (status != B_OK)
>         return status;
>
>     ...
> }
>
> if ((mode & B_STAT_GID) != 0) {
>     status_t status = CheckPermissions(W_OK, B_STAT_GID);
>     ...
> }
>
> Please review the patch, I hope it's conforms to the coding guidelines.
> After that I'll attach the same on Trac, and send a similar patch for
> ext2fs.
>
> Best Regards,
> Rohit Yadav
>
Index: src/add-ons/kernel/file_systems/ext2/Inode.h
===================================================================
--- src/add-ons/kernel/file_systems/ext2/Inode.h        (revision 39257)
+++ src/add-ons/kernel/file_systems/ext2/Inode.h        (working copy)
@@ -5,7 +5,6 @@
 #ifndef INODE_H
 #define INODE_H
 
-
 #include <fs_cache.h>
 #include <lock.h>
 #include <string.h>
@@ -45,7 +44,8 @@
                                                        { return 
S_ISREG(Mode()); }
                        bool            IsSymLink() const
                                                        { return 
S_ISLNK(Mode()); }
-                       status_t        CheckPermissions(int accessMode) const;
+                       status_t        CheckPermissions(int accessMode,
+                                                       uint32 statMask = 0) 
const;
 
                        bool            IsDeleted() const { return fUnlinked; }
                        bool            HasExtraAttributes() const
Index: src/add-ons/kernel/file_systems/ext2/Inode.cpp
===================================================================
--- src/add-ons/kernel/file_systems/ext2/Inode.cpp      (revision 39257)
+++ src/add-ons/kernel/file_systems/ext2/Inode.cpp      (working copy)
@@ -6,9 +6,9 @@
 
 #include "Inode.h"
 
-#include <fs_cache.h>
 #include <string.h>
 #include <util/AutoLock.h>
+#include <NodeMonitor.h>
 
 #include "CachedBlock.h"
 #include "DataStream.h"
@@ -185,7 +185,7 @@
 
 
 status_t
-Inode::CheckPermissions(int accessMode) const
+Inode::CheckPermissions(int accessMode, uint32 statMask) const
 {
        uid_t user = geteuid();
        gid_t group = getegid();
@@ -202,6 +202,12 @@
                        return B_OK;
        }
 
+       // users are always allowed to chmod() files they own
+       const uint32 ownership = B_STAT_MODE | B_STAT_GID | B_STAT_UID;
+       if ((statMask & ownership) && !(statMask & ~ownership) &&
+               (user == (uid_t)fNode.UserID()))
+                       return B_OK;
+
        // shift mode bits, to check directly against accessMode
        mode_t mode = Mode();
        if (user == (uid_t)fNode.UserID())
Index: src/add-ons/kernel/file_systems/ext2/kernel_interface.cpp
===================================================================
--- src/add-ons/kernel/file_systems/ext2/kernel_interface.cpp   (revision 39257)
+++ src/add-ons/kernel/file_systems/ext2/kernel_interface.cpp   (working copy)
@@ -580,7 +580,7 @@
 
        Inode* inode = (Inode*)_node->private_node;
 
-       status_t status = inode->CheckPermissions(W_OK);
+       status_t status = inode->CheckPermissions(W_OK, mask);
        if (status < B_OK)
                return status;

Other related posts: