Maxim Cournoyer schreef op ma 19-04-2021 om 08:07 [-0400]:
> Hi Maxime!
> 
> Maxime Devos <maximedevos@telenet.be> writes:
> 
> > Maxim Cournoyer schreef op za 17-04-2021 om 16:06 [-0400]:
> > +                      [...]
> > +                       (delete-file-recursively "/var/lib/jami/accounts"))
> > 
> > You might want to verify whether /var/lib/jami/{.cache,.config,.local/share,.local}
> > aren't symbolic links.  That way, if the Jami daemon is compromised (due to buffer
> > overflow --> arbitrary code execution or something), the attacker can't trick the
> > shepherd service into deleting arbitrary directories.
> 
> It would only be able to delete directories that are world writable
> though, right?  Seems the opportunity to cause damage is limited, but
> it's a simple check to add, so I'll do it.

Let's step through the relevant code of the shepherd service.

(shepherd-service
  (documentation "Run the Jami daemon.")
  [blabla]
  (start
    #~(lambda args
        [other stuff]
        (when [blabla, and a 'catch' form]
          (delete-file-recursively "/var/lib/jami/.cache/jami")
          [etcetera])
         (let* (([blabla])
                (user (passwd:uid [blabla jami user]))
                (group [likewise]))
           [blabla] (chown accounts-dir user group)))
        ;; Start the daemon
        (define daemon-pid
          (fork+exec-command [blabla Jami cmdline arguments]
            #:user "jami" #:group "jami" [blabla]))
        [blabla]))
   (stop [blabla]))         

Remember that the shepherd daemon is run as root (and therefore
has read-write-execute access to everything).  The 'start' procedure
(and 'stop' procedure) are run _within_ the shepherd daemon.  Thus, the
'start' gexp is run as root.

As the start procedure didn't change the uid/gid from root to something
else, (delete-file-recursively "/var/lib/jami/.cache/jami") is run as
root.  IIUC, root user can read/write anything, ignoring things like "user"
and "group".  World-writability is not required.

> What about if the daemon was
> run in a container (your suggestion in a following email, to which I
> agree would be a good thing)?  It would prevent this kind of attack,
> right?

I don't see how that would help.  It is the _shepherd daemon_ (that runs
as root) that runs (delete-file-recursively ...), not the attacker (from
within the compromised jami-daemon process).  Perhaps this is cleared up
by my previous response?  If not, please walk me through the attack scenario
you had in mind that would be thwarted by running the jami-daemon in a container.

> > Example scenario:
> >   * the jami daemon has a security bug that allows arbitrary code execution
> >     within the daemon
> >   * the attacker exploits this
> >   * now the attacker can modify everything under /var/lib/jami
> >   * the attacker deletes /var/lib/jami/.config and replaces it with a symlink
> >     to /home/ANY-USER/.config
> >   * eventually, the system reboots
> >   * (delete-file-recursively "/var/lib/jami/.config/jami") is run.
> >     As "/var/lib/jami/.config" points to "/home/ANY-USER/.config",
> >     this means "/home/ANY-USER/.config/jami" is deleted.
> >   * thus, ANY-USER loses their jami configuration
> 
> The cleanup code is run as the 'jami' user,

Err, the cleanup code is run from within the shepherd service, outside the
fork+exec-command.  The shepherd service is run _within_ the shepherd daemon,
which runs as *root*.  Only in the fork+exec-command, code is run as the 'jami'
user; the cleanup code is run as 'root'.

> so I don't think it'd be able to touch anything under /home/ANY-OTHER-USER/,
> unless they manually loosened permissions on their home directory (shooting
> themselves in the foot).
See my comment above.

Greetings,
Maxime.