Re: [PATCH 2/2] services.cgi: Fix status/actions on services with name != addon name

Hello,

I am going to start with the second patch, because that is going to be shorter :)

On 3 Oct 2022, at 16:27, Robin Roevens robin.roevens@disroot.org wrote:

• addonctrl's new functionality to control explicit addon services was

implemented.

• Change 'Addon' column header to 'Addon Service' to be clear that

it's not addons but services listed here.

• Services not matching the name of the addon now display the addon

name between parentheses, so the user knows where the service comes from.

• When no valid runlevel symlink is found by addonctrl for a service,

the 'enable on boot' checkbox is replaced by a small exclamation point with alt-text "No valid runlevel symlink was found for the initscript of this service." to inform user why a service can't be enabled.

• Added German and Dutch translation for above message.

Fixes: Bug#12935 Signed-off-by: Robin Roevens robin.roevens@disroot.org

---

html/cgi-bin/services.cgi | 103 +++++++++++++++----------------------- langs/de/cgi-bin/de.pl | 1 + langs/en/cgi-bin/en.pl | 1 + langs/nl/cgi-bin/nl.pl | 1 + 4 files changed, 42 insertions(+), 64 deletions(-)

diff --git a/html/cgi-bin/services.cgi b/html/cgi-bin/services.cgi index 29926ecc3..e91da884b 100644 --- a/html/cgi-bin/services.cgi +++ b/html/cgi-bin/services.cgi @@ -142,14 +142,14 @@ END my $paramstr=$ENV{QUERY_STRING}; my @param=split(/!/, $paramstr); if ($param[1] ne ''){

• &General::system("/usr/local/bin/addonctrl", "$param[0]", "$param[1]");
  

• &General::system("/usr/local/bin/addonctrl", "$param[0]", "$param[1]", "$param[2]");
  

  }

Although this has been like this before, I am strongly against passing anything from the request to the setuid binary.

This will open us up for any shell command injections and so on. I would prefer to check the second argument to be at least on some kind of list as there are only a few keywords allowed (start, stop, restart, …) and the other parameters should only contain ASCII letters, numbers and maybe dash and underscore. Nothing else should be passed through here.

print <<END

<div align='center'> <table width='80%' cellspacing='1' class='tbl'> <tr> - <th align='center'><b>Addon</b></th> + <th align='left'><b>Addon $Lang::tr{service}</b></th> <th align='center'><b>Boot</b></th> <th align='center' colspan=2><b>$Lang::tr{'action'}</b></th> <th align='center'><b>$Lang::tr{'status'}</b></th> @@ -170,33 +170,35 @@ END foreach my $pak (keys %paklist) { my %metadata = &Pakfire::getmetadata($pak, "installed"); + my $service; + if ("$metadata{'Services'}") { - foreach my $service (split(/ /, "$metadata{'Services'}")) { - push(@addon_services, $service); - } - } - } + foreach $service (split(/ /, "$metadata{'Services'}")) { + $lines++; + if ($lines % 2) { + print "<tr>"; + $col="bgcolor='$color{'color22'}'"; + } else { + print "<tr>"; + $col="bgcolor='$color{'color20'}'"; + }

• foreach (@addon_services) {

• $lines++;
  

• if ($lines % 2){
  

• 	print "<tr>";
  

• 	$col="bgcolor='$color{'color22'}'";
  

• }else{
  

• 	print "<tr>";
  

• 	$col="bgcolor='$color{'color20'}'";
  

• 		# Add addon name to displayname of service if servicename differs from addon
  

• 		my $displayname = ($pak ne $service) ? "$service ($pak)" : $service;
  

• 		print "<td align='left' $col width='31%'>$displayname</td> ";
  

• 		my $status = isautorun($pak,$service,$col);
  

• 		print "$status ";
  

• 		print "<td align='center' $col width='8%'><a href='services.cgi?$pak!start!$service'><img alt='$Lang::tr{'start'}' title='$Lang::tr{'start'}' src='/images/go-up.png' border='0' /></a></td>";
  

• 		print "<td align='center' $col width='8%'><a href='services.cgi?$pak!stop!$service'><img alt='$Lang::tr{'stop'}' title='$Lang::tr{'stop'}' src='/images/go-down.png' border='0' /></a></td> ";
  

• 		my $status = isrunningaddon($pak,$service,$col);
  

• 		$status =~ s/\\[[0-1]\;[0-9]+m//g;
  

• 		chomp($status);
  

• 		print "$status";
  

• 		print "</tr>";
  

• 	}
  

  }

• print "<td align='left' $col width='31%'>$_</td> ";
  

• my $status = isautorun($_,$col);
  

• print "$status ";
  

• print "<td align='center' $col width='8%'><a href='services.cgi?$_!start'><img alt='$Lang::tr{'start'}' title='$Lang::tr{'start'}' src='/images/go-up.png' border='0' /></a></td>";
  

• print "<td align='center' $col width='8%'><a href='services.cgi?$_!stop'><img alt='$Lang::tr{'stop'}' title='$Lang::tr{'stop'}' src='/images/go-down.png' border='0' /></a></td> ";
  

• my $status = isrunningaddon($_,$col);
  

• $status =~ s/\\[[0-1]\;[0-9]+m//g;
  

• chomp($status);
  

• print "$status";
  

• print "</tr>";
  

  }

  print "</table></div>\n";

@@ -215,51 +217,24 @@ END }

sub isautorun (@) {

• my ($cmd, $col) = @_;
• # Init directory.
• my $initdir = "/etc/rc.d/rc3.d/";
• my $status = "<td align='center' $col></td>";

• my ($pak, $service, $col) = @_;
• my @testcmd = &General::system_output("/usr/local/bin/addonctrl", "$pak", "boot-status", "$service");
• my $testcmd = @testcmd[0];
• my $status = "<td align='center' $col><img alt='$Lang::tr{'service boot setting unavailable'}' title='$Lang::tr{'service boot setting unavailable'}' src='/images/dialog-warning.png' border='0' width='16' height='16' /></td>";

• # Check if autorun for the given cmd is enabled.
• if ( &find_init("$cmd", "$initdir") ) {

• # Check if autorun for the given service is enabled.
• if ( $testcmd =~ /enabled\ on\ boot/ ) { # Adjust status.

• $status = "<td align='center' $col><a href='services.cgi?$_!disable'><img alt='$Lang::tr{'deactivate'}' title='$Lang::tr{'deactivate'}' src='/images/on.gif' border='0' width='16' height='16' /></a></td>";
  

• } else {

• $status = "<td align='center' $col><a href='services.cgi?$pak!disable!$service'><img alt='$Lang::tr{'deactivate'}' title='$Lang::tr{'deactivate'}' src='/images/on.gif' border='0' width='16' height='16' /></a></td>";
  

• } elsif ( $testcmd =~ /disabled\ on\ boot/ ) { # Adjust status.

• $status = "<td align='center' $col><a href='services.cgi?$_!enable'><img alt='$Lang::tr{'activate'}' title='$Lang::tr{'activate'}' src='/images/off.gif' border='0' width='16' height='16' /></a></td>";
  

• $status = "<td align='center' $col><a href='services.cgi?$pak!enable!$service'><img alt='$Lang::tr{'activate'}' title='$Lang::tr{'activate'}' src='/images/off.gif' border='0' width='16' height='16' /></a></td>";
  

  }

  # Return the status. return $status;

}

-sub find_init (@) {

• my ($cmd, $dir) = @_;
• # Open given init directory.
• opendir (INITDIR, "$dir") || die "Cannot opendir $dir: $!";
• # Read-in init files from directory.
• my @inits = readdir(INITDIR);
• # Close directory handle.
• closedir(INITDIR);
• # Loop through the directory.
• foreach my $init (@inits) {

• # Check if the current processed file belongs to the given command.
  

• if ($init =~ /S\d+\d+$cmd\z/) {
  

• 	# Found, return "1" - True.
  

• 	return "1";
  

• }
  

•    }
  

• # Nothing found, return nothing.
• return;

-}

sub isrunning (@) { my ($cmd, $col) = @_; my $status = "<td align='center' bgcolor='${Header::colourred}'><font color='white'><b>$Lang::tr{'stopped'}</b></font></td><td colspan='2' $col></td>"; @@ -313,7 +288,7 @@ sub isrunning (@) { }

sub isrunningaddon (@) {

• my ($cmd, $col) = @_;

• my ($pak, $service, $col) = @_;

  my $status = "<td align='center' bgcolor='${Header::colourred}'><font color='white'><b>$Lang::tr{'stopped'}</b></font></td><td colspan='2' $col></td>"; my $pid = '';

@@ -321,7 +296,7 @@ sub isrunningaddon (@) { my $exename; my @memory;

• my @testcmd = &General::system_output("/usr/local/bin/addonctrl", "$cmd", "status");

• my @testcmd = &General::system_output("/usr/local/bin/addonctrl", "$pak", "status", "$service"); my $testcmd = @testcmd[0];

  if ( $testcmd =~ /is\ running/ && $testcmd !~ /is\ not\ running/){

diff --git a/langs/de/cgi-bin/de.pl b/langs/de/cgi-bin/de.pl index 798abcffc..db7d117b0 100644 --- a/langs/de/cgi-bin/de.pl +++ b/langs/de/cgi-bin/de.pl @@ -2251,6 +2251,7 @@ 'server string' => 'Server String', 'service' => 'Dienst', 'service added' => 'Benutzerdefinierter Netzwerkdienst wurde hinzugefügt', +'service boot setting unavailable' => 'Für das Initscript dieses Dienstes wurde kein gültiger Runlevel-Symlink gefunden.', 'service name' => 'Name des Dienstes:', 'service removed' => 'Benutzerdefinierter Netzwerkdienst wurde entfernt', 'service updated' => 'Benutzerdefinierter Netzwerkdienst wurde aktualisiert', diff --git a/langs/en/cgi-bin/en.pl b/langs/en/cgi-bin/en.pl index f770e7cd9..60dca5be4 100644 --- a/langs/en/cgi-bin/en.pl +++ b/langs/en/cgi-bin/en.pl @@ -2306,6 +2306,7 @@ 'server string' => 'Server String', 'service' => 'Service', 'service added' => 'Custom network service added', +'service boot setting unavailable' => 'No valid runlevel symlink was found for the initscript of this service.', 'service name' => 'Service name:', 'service removed' => 'Custom network service removed', 'service updated' => 'Custom network service updated', diff --git a/langs/nl/cgi-bin/nl.pl b/langs/nl/cgi-bin/nl.pl index 49dabec99..4fd6955cc 100644 --- a/langs/nl/cgi-bin/nl.pl +++ b/langs/nl/cgi-bin/nl.pl @@ -1899,6 +1899,7 @@ 'server string' => 'Server String', 'service' => 'Dienst', 'service added' => 'Aangepaste netwerkdienst toegevoegd', +'service boot setting unavailable' => 'Er werd voor het initscript van deze service geen geldige runlevel symlink gevonden.', 'service name' => 'Dienstennaam:', 'service removed' => 'Aangepaste netwerkdienst verwijderd', 'service updated' => 'Aangepaste netwerkdienst bijgewerkt', -- 2.37.3

-- Dit bericht is gescanned op virussen en andere gevaarlijke inhoud door MailScanner en lijkt schoon te zijn.