Comment 1 Robert Clausecker 2023-09-10 23:31:18 UTC

Thank you for your submission.  Could you update this patch to the most recent version of dasherr?  Here are some issues I noticed during review:

 - please set LICENSE_FILE
 - instead of ${CP} -R, use one of the COPYTREE_... macros if possible
 - please provide a real pkg-plist.  Generating it in pre-install is not
   something we want (outside of certain well-established USES)
 - why do the files need to be owned by www?  This means the webserver
   will be able to write them, which is usually a bad thing

Please check and resubmit.

Created attachment 244814 [details]
dasherr-1.05.01.patch

Description:

* Update to 1.05.01
* Use static pkg-plist instead of dynamic

ChangeLog: https://github.com/erohtar/Dasherr/releases/tag/v1.05.01

* Fixed: Pressing save on alternate configuration pages saved that configuration over main config file.

QA:

* portlint: OK (looks fine.)
* testport: OK (poudriere: 13.2-RELEASE, amd64)

(In reply to Robert Clausecker from comment #1)

Hi Robert, thank you for review this port.

>  - please set LICENSE_FILE

It is not included in the distfile, but since it is a known license I have not put LICENSE_FILE.

>  - why do the files need to be owned by www?  This means the webserver will be able to write them, which is usually a bad thing

As in any other port I use www as generic user, all those projects usually need a web server like NGINX or apache that by default run as www user. If the user needs to modify this parameter, a chown will adapt to his needs.

```
# grep -e'@owner www' -e'@group www' www/*/pkg-plist | wc -l
      45
```

Some, like WordPress, generate the pkg-plist file dynamically but use www in the same way.

Of course, I listen to any other recommendations, but I think www is fine.

Comment 4 Robert Clausecker 2023-09-14 01:11:46 UTC

Thank you for informing me that other ports suffer from the same mistake.

The problem is as follows: files owned by www are writable by the http daemon (whichever it is).  So if there is a bug in the web application, an attacker can very likely use it to modify the web application itself, persisting the attack and possibly establishing a remote shell.  Thus, files that don't need to be writable by httpd must not be owned by www!  Only give files to www that httpd needs to write.  Ports that do this wrong have a possible security issue and should be fixed.

> Of course, I listen to any other recommendations, but I think www is fine.

No, it is not fine.  Please also fix your other ports if they make the same mistake.

(In reply to Robert Clausecker from comment #4)

I understand the problem. Yes, you are right, I thought www should be used in FreeBSD ports, but it is not. I will remove `@owner www` and `@group www` from pkg-plist and try again.

This is my first PHP port, so I don't have any other similar port to change.

Created attachment 244857 [details]
dasherr-1.05.01.patch

Description:

* Add settings.json

QA:

* portlint: OK (looks fine.)
* testport: OK (poudriere: 13.2-RELEASE, amd64)

Done, Robert. I have only set the owner (www) and group (www) of the settings.json file since the user can change it through the web interface.

Comment 8 Robert Clausecker 2023-09-15 00:08:18 UTC

(In reply to Jesús Daniel Colmenares Oviedo from comment #7)

Hi Jesús,

If the settings.json file is mutable, you should use the @sample mechanism to install it.  This saves the file from being overwritten on package updates.

For individual files, you can use syntax like

    @sample(www,www,) %%WWWDIR%%/settings.json.sample

to set owner and group, making the file a sample at the same time.  This is a bit more compact.

Created attachment 244864 [details]
dasherr-1.05.01.patch

Description:

* Use @sample instead of copying settings.json directly

QA:

* portlint: OK (looks fine.)
* testport: OK (poudriere: 13.2-RELEASE, amd64)

(In reply to Robert Clausecker from comment #8)

Thanks!

Comment 11 commit-hook 2023-09-17 15:31:20 UTC

A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/ports/commit/?id=a87df9468d4b3c40b33ff3572ee1647cd7d9974c

commit a87df9468d4b3c40b33ff3572ee1647cd7d9974c
Author:     Jesús Daniel Colmenares Oviedo <DtxdF@disroot.org>
AuthorDate: 2023-09-15 01:39:23 +0000
Commit:     Robert Clausecker <fuz@FreeBSD.org>
CommitDate: 2023-09-17 15:23:14 +0000

    www/dasherr: New port: Lightweight dashboard for self-hosted services (and bookmarks)

    Dasherr is a minimal and lightweight dashboard for your self-hosted
    services (and bookmarks).

    Features:

    * Loads instantly + Remains light on resources
    * Responsive design (uses Bootstrap framework)
    * Shows Temperature, CPU load and Memory used by tapping into Glances
      API (default 5s updates)
    * Built-in online check of services (checked only at time of page
      load/refresh, to minimize background activity & load)
    * Several built-in Themes (easy to edit & add your own)
    * Wallpaper backgrounds supported
    * FontAwesome icons (also supports Self-hosted/Web image icons)
    * All settings in a single easy to edit json file, with built-in editor
    * Support for alternate configurations without needing multiple Dasherr
      installations

    WWW: https://github.com/erohtar/Dasherr/

    PR:             272777

 www/Makefile                |  1 +
 www/dasherr/Makefile (new)  | 24 ++++++++++++++++++++++++
 www/dasherr/distinfo (new)  |  3 +++
 www/dasherr/pkg-descr (new) | 17 +++++++++++++++++
 www/dasherr/pkg-plist (new) | 19 +++++++++++++++++++
 5 files changed, 64 insertions(+)

Comment 12 Robert Clausecker 2023-09-17 15:35:16 UTC

Thank you for your contribution.