[SOLVED] - Is this C code safe to use ?

ViruSzZ · 2011-10-19 21:22:53

I'm not knowledgeable in C at all and I don't know what exactly this program does and is it safe to run it or not, but I'm trying to execute some commands via PHP's exec() which are failing due to lack of permissions. If I set the SGID bits the setuid is set on execution and the commands are succeeding but is this a safe way?? For example the following command is failing with a permission denied:

sed -r 's/(GNU\/Linux |release |\\|\([^()]*\))//g;q' /dev/$1/etc/issue

executed this way in PHP:

exec("bin/get_os " . $vserver, $osname);

So a mate gave me this C code saying it's some kind of a 'wrapper' which 'safely' executes programs with root privileges and that it is a safer way to use this wrapper through PHP's exec().

Can anyone tell me if this C program is OK and safe to use?

┌─[22:59]-[root@stavrovski]-[~]
└─› cat /tmp/wrapper.c 
/**
 * vim:ft=c:fenc=UTF-8:ts=2:sts=2:sw=2:expandtab
 *
 * Copyright 2011 TechnoGate <contact@technogate.fr>
 *
 * This program is free software: you can redistribute it and/or modify it under the
 * terms of the GNU General Public License as published by the Free Software
 * Foundation, either version 3 of the License, or (at your option) any later
 * version.
 *
 * This program is distributed in the hope that it will be useful, but WITHOUT ANY
 * WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR A
 * PARTICULAR PURPOSE. See the GNU General Public License for more details.
 *
 * You should have received a copy of the GNU General Public License along with
 * This program. If not, see http://www.gnu.org/licenses/
 */

#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <string.h>
#include <sys/types.h>
#include <sys/stat.h>
#include <pwd.h>

int main(int argc, char **argv)
{
  int    file_owner, status;
  char   *newenviron[] = { NULL };
  uid_t  current_uid;
  pid_t  child_pid, wpid;
  ushort file_mode;
  struct stat status_buf;
  struct passwd * passent;

  // Check that we are root!
  current_uid = geteuid();
  if(current_uid != 0) {
    fprintf(stderr, "The wrapper should be ran as root, you should maybe set the owner to root and set setuid.\n");
    return 1;
  }

  // Check argc
  if (argc < 2) {
    fprintf(stderr, "Usage: %s <script> [arguments]\n", argv[0]);
    return 1;
  }

  // Make sure the script does exist
  if(fopen(argv[1], "r") == NULL) {
    fprintf(stderr, "The file %s does not exist.\n", argv[1]);
    return 1;
  }

  // Get the attributes of the file
  if(stat(argv[1], &status_buf) < 0) {
    fprintf(stderr, "Can't stat %s\n", argv[1]);
    return 1;
  }

  // Get the permissions of the file
  file_mode = status_buf.st_mode;

  // Make sure it's executable and it's setuid
  if( !( (file_mode & S_ISUID) || (file_mode & S_IXUSR) ) ) {
    fprintf(stderr, "The file %s should be executable and should have setuid set.\n", argv[1]);
    return 1;
  }

  // Get the owner of the script
  file_owner = status_buf.st_uid;

  // Get the name of the owner
  passent = getpwuid(file_owner);

  // setuid
  if(setuid(file_owner)) {
    if(passent != NULL)
      fprintf(stderr, "Can't change the persona to %s\n", passent -> pw_name);
    else
      fprintf(stderr, "Can't change the persona to %d\n", file_owner);
    return 1;
  }

  // Fork
  child_pid = fork();
  if(child_pid == -1) {
    perror("fork");
    exit(EXIT_FAILURE);
  }

  // Execute the command in a child
  if(child_pid == 0) {
    execve(argv[1], argv + 1, newenviron);
    perror("Failed to execute the script");
    exit(EXIT_FAILURE);
  } else {
    do {
      wpid = waitpid(child_pid, &status, WUNTRACED
    #ifdef WCONTINUED       /* Not all implementations support this */
        | WCONTINUED
    #endif
        );

      // Seems the child is not running
      if (wpid == -1) {
        perror("waitpid");
        exit(EXIT_FAILURE);
      }

      if (WIFEXITED(status)) { /* Child exited ? */
        return WEXITSTATUS(status);


      } else if (WIFSIGNALED(status)) { /* Child signaled ? */
        return WTERMSIG(status);

      } else if (WIFSTOPPED(status)) { /* Child stopped */
        return WSTOPSIG(status);


    #ifdef WIFCONTINUED     /* Not all implementations support this */
      } else if (WIFCONTINUED(status)) {
        // The child is still running...
    #endif
      } else {    /* Non-standard case -- may never happen */
        printf("Unexpected status (0x%x)\n", status);
      }
    } while (!WIFEXITED(status) && !WIFSIGNALED(status));
  }

  return EXIT_FAILURE;
}

Last edited by ViruSzZ (2011-10-20 20:12:31)

fukawi2 · 2011-10-19 22:23:00

It *seems* "safe" although I don't see how it makes executing the program any safer than just calling the program directly.

maddog39 · 2011-10-20 19:12:16

Yup seems okay to me as well though don't see the point of it in your case. Might as well run it from PHP and if you really want to fork and exec, do it in PHP.

ViruSzZ · 2011-10-20 20:12:11

Ok, thanks for your inputs, much appreciated. Anyhow, the point of this code as my mate said and explained to me today again, was intended to run an external script with only one argument passed to it via PHP using the wrapper and yes, i totally don't need it as I'm executing things directly from PHP.

Again, thanks for your thoughts, take care archers

I'm marking this as solved.

Arch Linux

#1 2011-10-19 21:22:53

[SOLVED] - Is this C code safe to use ?

#2 2011-10-19 22:23:00

Re: [SOLVED] - Is this C code safe to use ?

#3 2011-10-20 19:12:16

Re: [SOLVED] - Is this C code safe to use ?

#4 2011-10-20 20:12:11

Re: [SOLVED] - Is this C code safe to use ?

Board footer