~mil/sxmo-tickets#128: 
Use file permissions instead of setuid

setpineled, setpinebacklight, and screenlock currently bypass system permissions by always running as root. This is a serious security concern as compromising one of these programs grants privilege escalation.

These currently need root because they write to protected files in sysfs. A better solution would be to make an openrc boot script that makes these files writable by everyone.

Status
RESOLVED FIXED
Submitter
~kgp445
Assigned to
No-one
Submitted
3 years ago
Updated
3 years ago
Labels
No labels applied.

~kgp445 3 years ago

I have made a patch, but it will require tweaks to the upstream ABUILD as well and I'm not sure how to build a test package instead of pulling a remote release archive.

diff --git a/configs/openrc/sxmo-sysfs-perms b/configs/openrc/sxmo-sysfs-perms new file mode 100644 index 0000000..621c67b --- /dev/null +++ b/configs/openrc/sxmo-sysfs-perms @@ -0,0 +1,29 @@ +#!/sbin/openrc-run + +description="Makes some sysfs files writable for unprivileged software" +command="/bin/chmod" +command_args="a+w

  • /sys/power/mem_sleep
  • /sys/power/state
  • /sys/bus/usb/drivers/usb/unbind
  • /sys/bus/usb/drivers/usb/bind" +extra_commands="pp pbp"
  • +echmod() {
  • [ -e "$1" ] && chmod a+w "$1" +}
  • +pbp() {
  • echmod /sys/class/backlight/edp-backlight/brightness
  • echmod "/sys/class/wakeup/*/device/power/wakeup" +}
  • +pp() {
  • echmod /sys/devices/platform/backlight/backlight/backlight/brightness
  • echmod /sys/devices/platform/gpio-keys/power/wakeup
  • echmod /sys/devices/platform/soc/1c28c00.serial/serial1/serial1-0/power/wakeup
  • echmod /sys/devices/platform/soc/1f00000.rtc/power/wakeup
  • echmod /sys/devices/platform/soc/1f03400.rsb/sunxi-rsb-3a3/axp221-pek/power/wakeup
  • echmod "/sys/class/leds/*/brightness"
  • echmod "/sys/class/wakeup/*/device/power/wakeup" +} diff --git a/programs/sxmo_screenlock.c b/programs/sxmo_screenlock.c index dc46a89..fb9530b 100644 --- a/programs/sxmo_screenlock.c +++ b/programs/sxmo_screenlock.c @@ -472,8 +472,6 @@ main(int argc, char **argv) { } }
  • if (setuid(0))
  •   die("setuid(0) failed\n");
    
    if (!(dpy = XOpenDisplay(NULL))) die("Cannot open display\n");

diff --git a/programs/sxmo_setpinebacklight.c b/programs/sxmo_setpinebacklight.c index 0352c82..ea5a7a9 100644 --- a/programs/sxmo_setpinebacklight.c +++ b/programs/sxmo_setpinebacklight.c @@ -28,11 +28,6 @@ int main(int argc, char *argv[]) { argc--; brightness = atoi(argv[argc--]);

  • if (setuid(0)) {
  •   fprintf(stderr, "setuid(0) failed\n");
    
  •   return 1;
    
  • }
  • if (access(pbpScreen, F_OK) != -1) { writeFile(pbpScreen, brightness); fprintf(stderr, "Set PBP brightness to %d\n", brightness); diff --git a/programs/sxmo_setpineled.c b/programs/sxmo_setpineled.c index 97a05a9..b86bd54 100644 --- a/programs/sxmo_setpineled.c +++ b/programs/sxmo_setpineled.c @@ -48,9 +48,5 @@ int main(int argc, char *argv[]) { "sh -c 'echo %d > /sys/class/leds/%s:%s/brightness'", brightness, color, type );
  • if (setuid(0)) {
  •   fprintf(stderr, "setuid(0) failed\n");
    
  • } else {
  •   return system(command);
    
  • }
  • return system(command); }

~kgp445 3 years ago

Ah, I see that I should not post patches through the website...

~kgp445 3 years ago

Hm, it seems that some of these sysfs paths (all copied from the related C files) actually do not exist on my system.

~proycon referenced this from #159 3 years ago

~proycon REPORTED FIXED 3 years ago

Register here or Log in to comment, or comment via email.