Avoid command injection with system() api [closed]

Clash Royale CLAN TAG#URR8PPP

up vote
5
down vote

favorite

We have a legacy C code used to allow less privileged user to run custom scripts with escalated privilege. This has SUID bit set. This code restricts the PATH env to a specific folder and then uses system() api to execute the script with restricted shell :

/bin/bash -r -c "script <arg>"

As the path is restricted, it can execute only scripts from that specific folder.

Now knowing all the pitfalls for command injection with system() api, what measures can be taken to avoid command injection? This is used in many places in various scripts etc, so don't want to do a completely new implementation to avoid any regression.

linux c

share|improve this question

edited Aug 27 at 5:05

Archemar

19k93366

asked Aug 27 at 4:50

Rajesh

283

closed as unclear what you're asking by Rui F Ribeiro, muru, schily, Romeo Ninov, andcoz Aug 27 at 15:21

Please clarify your specific problem or add additional details to highlight exactly what you need. As it's currently written, it’s hard to tell exactly what you're asking. See the How to Ask page for help clarifying this question. If this question can be reworded to fit the rules in the help center, please edit the question.

• 
  
  
  

  
  

  
  
  
  
  
  

  
  Whit the lack of data you present this question, the only sound advice we can provide is avoiding abusing suid. Otherwise, we cannot guess what your script does.
  – Rui F Ribeiro
  Aug 27 at 7:47
  
  

  
  
  
  


• 
  
  
  

  
  1
  

  
  
  
  
  
  

  
  An application that does not filter input has to be modified to filter input, no way to get around it with magical Unix solutions.
  – Rui F Ribeiro
  Aug 27 at 8:50
  

  
  
  
  


• 
  
  
  

  
  1
  

  
  
  
  
  
  

  
  As others have warned, it's easy to make mistakes in your own suid code. What about LD_PRELOAD and LD_LIBRARY_PATH, for example?
  – Edheldil
  Aug 27 at 9:06
  

  
  
  
  


• 
  
  
  

  
  

  
  
  
  
  
  

  
  Are you sure you need system (which runs a command in a shell) instead of exec (which runs a command)? Often it can be more useful and secure not to use system, even when losing some flexibility.
  – allo
  Aug 27 at 11:13
  

  
  
  
  

add a comment | 

up vote
5
down vote

favorite

We have a legacy C code used to allow less privileged user to run custom scripts with escalated privilege. This has SUID bit set. This code restricts the PATH env to a specific folder and then uses system() api to execute the script with restricted shell :

/bin/bash -r -c "script <arg>"

As the path is restricted, it can execute only scripts from that specific folder.

Now knowing all the pitfalls for command injection with system() api, what measures can be taken to avoid command injection? This is used in many places in various scripts etc, so don't want to do a completely new implementation to avoid any regression.

linux c

share|improve this question

edited Aug 27 at 5:05

Archemar

19k93366

asked Aug 27 at 4:50

Rajesh

283

closed as unclear what you're asking by Rui F Ribeiro, muru, schily, Romeo Ninov, andcoz Aug 27 at 15:21

Please clarify your specific problem or add additional details to highlight exactly what you need. As it's currently written, it’s hard to tell exactly what you're asking. See the How to Ask page for help clarifying this question. If this question can be reworded to fit the rules in the help center, please edit the question.

• 
  
  
  

  
  

  
  
  
  
  
  

  
  Whit the lack of data you present this question, the only sound advice we can provide is avoiding abusing suid. Otherwise, we cannot guess what your script does.
  – Rui F Ribeiro
  Aug 27 at 7:47
  
  

  
  
  
  


• 
  
  
  

  
  1
  

  
  
  
  
  
  

  
  An application that does not filter input has to be modified to filter input, no way to get around it with magical Unix solutions.
  – Rui F Ribeiro
  Aug 27 at 8:50
  

  
  
  
  


• 
  
  
  

  
  1
  

  
  
  
  
  
  

  
  As others have warned, it's easy to make mistakes in your own suid code. What about LD_PRELOAD and LD_LIBRARY_PATH, for example?
  – Edheldil
  Aug 27 at 9:06
  

  
  
  
  


• 
  
  
  

  
  

  
  
  
  
  
  

  
  Are you sure you need system (which runs a command in a shell) instead of exec (which runs a command)? Often it can be more useful and secure not to use system, even when losing some flexibility.
  – allo
  Aug 27 at 11:13
  

  
  
  
  

add a comment | 

up vote
5
down vote

favorite

up vote
5
down vote

favorite

We have a legacy C code used to allow less privileged user to run custom scripts with escalated privilege. This has SUID bit set. This code restricts the PATH env to a specific folder and then uses system() api to execute the script with restricted shell :

/bin/bash -r -c "script <arg>"

As the path is restricted, it can execute only scripts from that specific folder.

Now knowing all the pitfalls for command injection with system() api, what measures can be taken to avoid command injection? This is used in many places in various scripts etc, so don't want to do a completely new implementation to avoid any regression.

linux c

share|improve this question

edited Aug 27 at 5:05

Archemar

19k93366

asked Aug 27 at 4:50

Rajesh

283

We have a legacy C code used to allow less privileged user to run custom scripts with escalated privilege. This has SUID bit set. This code restricts the PATH env to a specific folder and then uses system() api to execute the script with restricted shell :

/bin/bash -r -c "script <arg>"

As the path is restricted, it can execute only scripts from that specific folder.

Now knowing all the pitfalls for command injection with system() api, what measures can be taken to avoid command injection? This is used in many places in various scripts etc, so don't want to do a completely new implementation to avoid any regression.

linux c

share|improve this question

edited Aug 27 at 5:05

Archemar

19k93366

asked Aug 27 at 4:50

Rajesh

283

share|improve this question

share|improve this question

edited Aug 27 at 5:05

Archemar

19k93366

edited Aug 27 at 5:05

Archemar

19k93366

edited Aug 27 at 5:05

Archemar

19k93366

19k93366

asked Aug 27 at 4:50

Rajesh

283

asked Aug 27 at 4:50

Rajesh

283

asked Aug 27 at 4:50

Rajesh

283

283

closed as unclear what you're asking by Rui F Ribeiro, muru, schily, Romeo Ninov, andcoz Aug 27 at 15:21

Please clarify your specific problem or add additional details to highlight exactly what you need. As it's currently written, it’s hard to tell exactly what you're asking. See the How to Ask page for help clarifying this question. If this question can be reworded to fit the rules in the help center, please edit the question.

closed as unclear what you're asking by Rui F Ribeiro, muru, schily, Romeo Ninov, andcoz Aug 27 at 15:21

Please clarify your specific problem or add additional details to highlight exactly what you need. As it's currently written, it’s hard to tell exactly what you're asking. See the How to Ask page for help clarifying this question. If this question can be reworded to fit the rules in the help center, please edit the question.

• 
  
  
  

  
  

  
  
  
  
  
  

  
  Whit the lack of data you present this question, the only sound advice we can provide is avoiding abusing suid. Otherwise, we cannot guess what your script does.
  – Rui F Ribeiro
  Aug 27 at 7:47
  
  

  
  
  
  


• 
  
  
  

  
  1
  

  
  
  
  
  
  

  
  An application that does not filter input has to be modified to filter input, no way to get around it with magical Unix solutions.
  – Rui F Ribeiro
  Aug 27 at 8:50
  

  
  
  
  


• 
  
  
  

  
  1
  

  
  
  
  
  
  

  
  As others have warned, it's easy to make mistakes in your own suid code. What about LD_PRELOAD and LD_LIBRARY_PATH, for example?
  – Edheldil
  Aug 27 at 9:06
  

  
  
  
  


• 
  
  
  

  
  

  
  
  
  
  
  

  
  Are you sure you need system (which runs a command in a shell) instead of exec (which runs a command)? Often it can be more useful and secure not to use system, even when losing some flexibility.
  – allo
  Aug 27 at 11:13
  

  
  
  
  

add a comment | 

• 
  
  
  

  
  

  
  
  
  
  
  

  
  Whit the lack of data you present this question, the only sound advice we can provide is avoiding abusing suid. Otherwise, we cannot guess what your script does.
  – Rui F Ribeiro
  Aug 27 at 7:47
  
  

  
  
  
  


• 
  
  
  

  
  1
  

  
  
  
  
  
  

  
  An application that does not filter input has to be modified to filter input, no way to get around it with magical Unix solutions.
  – Rui F Ribeiro
  Aug 27 at 8:50
  

  
  
  
  


• 
  
  
  

  
  1
  

  
  
  
  
  
  

  
  As others have warned, it's easy to make mistakes in your own suid code. What about LD_PRELOAD and LD_LIBRARY_PATH, for example?
  – Edheldil
  Aug 27 at 9:06
  

  
  
  
  


• 
  
  
  

  
  

  
  
  
  
  
  

  
  Are you sure you need system (which runs a command in a shell) instead of exec (which runs a command)? Often it can be more useful and secure not to use system, even when losing some flexibility.
  – allo
  Aug 27 at 11:13
  

  
  
  
  

Whit the lack of data you present this question, the only sound advice we can provide is avoiding abusing suid. Otherwise, we cannot guess what your script does.
– Rui F Ribeiro
Aug 27 at 7:47

Whit the lack of data you present this question, the only sound advice we can provide is avoiding abusing suid. Otherwise, we cannot guess what your script does.
– Rui F Ribeiro
Aug 27 at 7:47

1

1

An application that does not filter input has to be modified to filter input, no way to get around it with magical Unix solutions.
– Rui F Ribeiro
Aug 27 at 8:50

An application that does not filter input has to be modified to filter input, no way to get around it with magical Unix solutions.
– Rui F Ribeiro
Aug 27 at 8:50

1

1

As others have warned, it's easy to make mistakes in your own suid code. What about LD_PRELOAD and LD_LIBRARY_PATH, for example?
– Edheldil
Aug 27 at 9:06

As others have warned, it's easy to make mistakes in your own suid code. What about LD_PRELOAD and LD_LIBRARY_PATH, for example?
– Edheldil
Aug 27 at 9:06

Are you sure you need system (which runs a command in a shell) instead of exec (which runs a command)? Often it can be more useful and secure not to use system, even when losing some flexibility.
– allo
Aug 27 at 11:13

Are you sure you need system (which runs a command in a shell) instead of exec (which runs a command)? Often it can be more useful and secure not to use system, even when losing some flexibility.
– allo
Aug 27 at 11:13

add a comment | 

2 Answers
2

active

oldest

votes

up vote
6
down vote



accepted

Because its hard to get right, I'd suggest removing the SUID on your code. Change your C code to use sudo. By using sudo the harder aspects getting secure system programming are done.

Then you can carefully construct a sudo configuration, using visudo, that does the bare minimum required to perform the task and constrain this to the required users/groups. After configuring sudo, get someone other than you to test it and try to break the intended constrains.

share|improve this answer

edited Aug 27 at 8:52

answered Aug 27 at 5:44

danblack

2194

• 
  
  
  

  
  2
  

  
  
  
  
  
  

  
  I do agree with not abusing SUID. However, sudo does neither corrects automagically layer 7 problems, nor is a magical bullet to sloppy coding pratices. I actually find your answer just giving a dangerous sense of security.
  – Rui F Ribeiro
  Aug 27 at 7:45
  
  

  
  
  
  


• 
  
  
  

  
  

  
  
  
  
  
  

  
  happy to make changes. General premise is that sudo is better audited than code you write yourself. I think its easier to write sudoers config files correctly than code that calls system and is suid. I agree mistakes still can be made. I'll reword so it doesn't sound so flippant.
  – danblack
  Aug 27 at 8:48
  

  
  
  
  


• 
  
  
  

  
  1
  

  
  
  
  
  
  

  
  I am not saying it is entirely a bad idea, do not get me wrong. It just it is not enough to make it automatically secure. An application that does not filter input has to be modified to filter input, no way to get around it with magical Unix solutions. The OP is expecting a miracle that won't happen.
  – Rui F Ribeiro
  Aug 27 at 8:49
  
  

  
  
  
  


• 
  
  
  

  
  

  
  
  
  
  
  

  
  @danblack I think this would work except that I may not know all the user names upfront. Let me see if I can find a way to map all these users to few groups and sudo policy accordingly.
  – Rajesh
  Aug 27 at 9:21
  

  
  
  
  


• 
  
  
  

  
  

  
  
  
  
  
  

  
  Scripts in the restricted folders are properly validating and sanitizing the input. The issue that I was trying to address is, avoiding injection to what is passed to system() call. With sudo conf changes, I can change the code from /bin/bash -r -c <name-of-restricted-script> <arg> to - /bin/sudo -u admin <path-to-restricted-script> <arg>. Even if some one inject a command/script, it would either be executed with the original user permission or won't execute at all depending on permission of that script/command.
  – Rajesh
  Aug 27 at 9:29
  

  
  
  
  

 | 
show 1 more comment

up vote
2
down vote

Code injection requires the ability of the user to pass arbitrary strings as parameters to the system() call. This is pretty similar to SQL injections and should be avoided in a similar way: don't pass any user-defined string directly to the call:

• numerical parameters should be converted to integers, and then converted back to strings at the time of the call




• parameters which belong to a fixed dictionary should be converted to "enum" values or similar, then back to strings at the time of the call




• free text input should be restricted to inoffensive character set where possible (e.g. [a-zA-Z0-9]*). Where problematic characters (including space) are required, proper escaping should be applied (that is, a b should be replaced by a b, etc.)

share|improve this answer

answered Aug 27 at 9:49

Dmitry Grigoryev

4,937644

• 
  
  
  

  
  2
  

  
  
  
  
  
  

  
  backslash should be avoided for quoting as its encoding is part of some characters in some locale. Only single quotes are relatively safe. Note system() runs sh and the OP seems to want to run bash on top of that, so there may need to be two levels of quoting. Some sh implementations drop privileges when run setuid, so it may not work anyway. In any case, the environment would have to be sanitized as well.
  – Stéphane Chazelas
  Aug 27 at 10:46
  
  

  
  
  
  


• 
  
  
  

  
  

  
  
  
  
  
  

  
  DOn't forget to also run these checks on environment variables, and check all of them, not only PATH, also add a whitelist
  – Ferrybig
  Aug 27 at 10:58
  

  
  
  
  


• 
  
  
  

  
  1
  

  
  
  
  
  
  

  
  @Ferrybig, the only reasonable approach for the environment is to start with an empty one, and white list the ones that have harmless names and values and that may be needed like sudo does. Even then, there's harm that can be done with all the LC* ones or TZ for instance.
  – Stéphane Chazelas
  Aug 27 at 11:03
  

  
  
  
  

add a comment | 

2 Answers
2

active

oldest

votes

2 Answers
2

active

oldest

votes

active

oldest

votes

active

oldest

votes

up vote
6
down vote



accepted

Because its hard to get right, I'd suggest removing the SUID on your code. Change your C code to use sudo. By using sudo the harder aspects getting secure system programming are done.

Then you can carefully construct a sudo configuration, using visudo, that does the bare minimum required to perform the task and constrain this to the required users/groups. After configuring sudo, get someone other than you to test it and try to break the intended constrains.

share|improve this answer

edited Aug 27 at 8:52

answered Aug 27 at 5:44

danblack

2194

• 
  
  
  

  
  2
  

  
  
  
  
  
  

  
  I do agree with not abusing SUID. However, sudo does neither corrects automagically layer 7 problems, nor is a magical bullet to sloppy coding pratices. I actually find your answer just giving a dangerous sense of security.
  – Rui F Ribeiro
  Aug 27 at 7:45
  
  

  
  
  
  


• 
  
  
  

  
  

  
  
  
  
  
  

  
  happy to make changes. General premise is that sudo is better audited than code you write yourself. I think its easier to write sudoers config files correctly than code that calls system and is suid. I agree mistakes still can be made. I'll reword so it doesn't sound so flippant.
  – danblack
  Aug 27 at 8:48
  

  
  
  
  


• 
  
  
  

  
  1
  

  
  
  
  
  
  

  
  I am not saying it is entirely a bad idea, do not get me wrong. It just it is not enough to make it automatically secure. An application that does not filter input has to be modified to filter input, no way to get around it with magical Unix solutions. The OP is expecting a miracle that won't happen.
  – Rui F Ribeiro
  Aug 27 at 8:49
  
  

  
  
  
  


• 
  
  
  

  
  

  
  
  
  
  
  

  
  @danblack I think this would work except that I may not know all the user names upfront. Let me see if I can find a way to map all these users to few groups and sudo policy accordingly.
  – Rajesh
  Aug 27 at 9:21
  

  
  
  
  


• 
  
  
  

  
  

  
  
  
  
  
  

  
  Scripts in the restricted folders are properly validating and sanitizing the input. The issue that I was trying to address is, avoiding injection to what is passed to system() call. With sudo conf changes, I can change the code from /bin/bash -r -c <name-of-restricted-script> <arg> to - /bin/sudo -u admin <path-to-restricted-script> <arg>. Even if some one inject a command/script, it would either be executed with the original user permission or won't execute at all depending on permission of that script/command.
  – Rajesh
  Aug 27 at 9:29
  

  
  
  
  

 | 
show 1 more comment

up vote
6
down vote



accepted

Because its hard to get right, I'd suggest removing the SUID on your code. Change your C code to use sudo. By using sudo the harder aspects getting secure system programming are done.

Then you can carefully construct a sudo configuration, using visudo, that does the bare minimum required to perform the task and constrain this to the required users/groups. After configuring sudo, get someone other than you to test it and try to break the intended constrains.

share|improve this answer

edited Aug 27 at 8:52

answered Aug 27 at 5:44

danblack

2194

• 
  
  
  

  
  2
  

  
  
  
  
  
  

  
  I do agree with not abusing SUID. However, sudo does neither corrects automagically layer 7 problems, nor is a magical bullet to sloppy coding pratices. I actually find your answer just giving a dangerous sense of security.
  – Rui F Ribeiro
  Aug 27 at 7:45
  
  

  
  
  
  


• 
  
  
  

  
  

  
  
  
  
  
  

  
  happy to make changes. General premise is that sudo is better audited than code you write yourself. I think its easier to write sudoers config files correctly than code that calls system and is suid. I agree mistakes still can be made. I'll reword so it doesn't sound so flippant.
  – danblack
  Aug 27 at 8:48
  

  
  
  
  


• 
  
  
  

  
  1
  

  
  
  
  
  
  

  
  I am not saying it is entirely a bad idea, do not get me wrong. It just it is not enough to make it automatically secure. An application that does not filter input has to be modified to filter input, no way to get around it with magical Unix solutions. The OP is expecting a miracle that won't happen.
  – Rui F Ribeiro
  Aug 27 at 8:49
  
  

  
  
  
  


• 
  
  
  

  
  

  
  
  
  
  
  

  
  @danblack I think this would work except that I may not know all the user names upfront. Let me see if I can find a way to map all these users to few groups and sudo policy accordingly.
  – Rajesh
  Aug 27 at 9:21
  

  
  
  
  


• 
  
  
  

  
  

  
  
  
  
  
  

  
  Scripts in the restricted folders are properly validating and sanitizing the input. The issue that I was trying to address is, avoiding injection to what is passed to system() call. With sudo conf changes, I can change the code from /bin/bash -r -c <name-of-restricted-script> <arg> to - /bin/sudo -u admin <path-to-restricted-script> <arg>. Even if some one inject a command/script, it would either be executed with the original user permission or won't execute at all depending on permission of that script/command.
  – Rajesh
  Aug 27 at 9:29
  

  
  
  
  

 | 
show 1 more comment

up vote
6
down vote



accepted

up vote
6
down vote



accepted

Because its hard to get right, I'd suggest removing the SUID on your code. Change your C code to use sudo. By using sudo the harder aspects getting secure system programming are done.

Then you can carefully construct a sudo configuration, using visudo, that does the bare minimum required to perform the task and constrain this to the required users/groups. After configuring sudo, get someone other than you to test it and try to break the intended constrains.

share|improve this answer

edited Aug 27 at 8:52

answered Aug 27 at 5:44

danblack

2194

Because its hard to get right, I'd suggest removing the SUID on your code. Change your C code to use sudo. By using sudo the harder aspects getting secure system programming are done.

Then you can carefully construct a sudo configuration, using visudo, that does the bare minimum required to perform the task and constrain this to the required users/groups. After configuring sudo, get someone other than you to test it and try to break the intended constrains.

share|improve this answer

edited Aug 27 at 8:52

answered Aug 27 at 5:44

danblack

2194

share|improve this answer

share|improve this answer

edited Aug 27 at 8:52

edited Aug 27 at 8:52

edited Aug 27 at 8:52

answered Aug 27 at 5:44

danblack

2194

answered Aug 27 at 5:44

danblack

2194

answered Aug 27 at 5:44

danblack

2194

2194

• 
  
  
  

  
  2
  

  
  
  
  
  
  

  
  I do agree with not abusing SUID. However, sudo does neither corrects automagically layer 7 problems, nor is a magical bullet to sloppy coding pratices. I actually find your answer just giving a dangerous sense of security.
  – Rui F Ribeiro
  Aug 27 at 7:45
  
  

  
  
  
  


• 
  
  
  

  
  

  
  
  
  
  
  

  
  happy to make changes. General premise is that sudo is better audited than code you write yourself. I think its easier to write sudoers config files correctly than code that calls system and is suid. I agree mistakes still can be made. I'll reword so it doesn't sound so flippant.
  – danblack
  Aug 27 at 8:48
  

  
  
  
  


• 
  
  
  

  
  1
  

  
  
  
  
  
  

  
  I am not saying it is entirely a bad idea, do not get me wrong. It just it is not enough to make it automatically secure. An application that does not filter input has to be modified to filter input, no way to get around it with magical Unix solutions. The OP is expecting a miracle that won't happen.
  – Rui F Ribeiro
  Aug 27 at 8:49
  
  

  
  
  
  


• 
  
  
  

  
  

  
  
  
  
  
  

  
  @danblack I think this would work except that I may not know all the user names upfront. Let me see if I can find a way to map all these users to few groups and sudo policy accordingly.
  – Rajesh
  Aug 27 at 9:21
  

  
  
  
  


• 
  
  
  

  
  

  
  
  
  
  
  

  
  Scripts in the restricted folders are properly validating and sanitizing the input. The issue that I was trying to address is, avoiding injection to what is passed to system() call. With sudo conf changes, I can change the code from /bin/bash -r -c <name-of-restricted-script> <arg> to - /bin/sudo -u admin <path-to-restricted-script> <arg>. Even if some one inject a command/script, it would either be executed with the original user permission or won't execute at all depending on permission of that script/command.
  – Rajesh
  Aug 27 at 9:29
  

  
  
  
  

 | 
show 1 more comment

• 
  
  
  

  
  2
  

  
  
  
  
  
  

  
  I do agree with not abusing SUID. However, sudo does neither corrects automagically layer 7 problems, nor is a magical bullet to sloppy coding pratices. I actually find your answer just giving a dangerous sense of security.
  – Rui F Ribeiro
  Aug 27 at 7:45
  
  

  
  
  
  


• 
  
  
  

  
  

  
  
  
  
  
  

  
  happy to make changes. General premise is that sudo is better audited than code you write yourself. I think its easier to write sudoers config files correctly than code that calls system and is suid. I agree mistakes still can be made. I'll reword so it doesn't sound so flippant.
  – danblack
  Aug 27 at 8:48
  

  
  
  
  


• 
  
  
  

  
  1
  

  
  
  
  
  
  

  
  I am not saying it is entirely a bad idea, do not get me wrong. It just it is not enough to make it automatically secure. An application that does not filter input has to be modified to filter input, no way to get around it with magical Unix solutions. The OP is expecting a miracle that won't happen.
  – Rui F Ribeiro
  Aug 27 at 8:49
  
  

  
  
  
  


• 
  
  
  

  
  

  
  
  
  
  
  

  
  @danblack I think this would work except that I may not know all the user names upfront. Let me see if I can find a way to map all these users to few groups and sudo policy accordingly.
  – Rajesh
  Aug 27 at 9:21
  

  
  
  
  


• 
  
  
  

  
  

  
  
  
  
  
  

  
  Scripts in the restricted folders are properly validating and sanitizing the input. The issue that I was trying to address is, avoiding injection to what is passed to system() call. With sudo conf changes, I can change the code from /bin/bash -r -c <name-of-restricted-script> <arg> to - /bin/sudo -u admin <path-to-restricted-script> <arg>. Even if some one inject a command/script, it would either be executed with the original user permission or won't execute at all depending on permission of that script/command.
  – Rajesh
  Aug 27 at 9:29
  

  
  
  
  

2

2

I do agree with not abusing SUID. However, sudo does neither corrects automagically layer 7 problems, nor is a magical bullet to sloppy coding pratices. I actually find your answer just giving a dangerous sense of security.
– Rui F Ribeiro
Aug 27 at 7:45

I do agree with not abusing SUID. However, sudo does neither corrects automagically layer 7 problems, nor is a magical bullet to sloppy coding pratices. I actually find your answer just giving a dangerous sense of security.
– Rui F Ribeiro
Aug 27 at 7:45

happy to make changes. General premise is that sudo is better audited than code you write yourself. I think its easier to write sudoers config files correctly than code that calls system and is suid. I agree mistakes still can be made. I'll reword so it doesn't sound so flippant.
– danblack
Aug 27 at 8:48

happy to make changes. General premise is that sudo is better audited than code you write yourself. I think its easier to write sudoers config files correctly than code that calls system and is suid. I agree mistakes still can be made. I'll reword so it doesn't sound so flippant.
– danblack
Aug 27 at 8:48

1

1

I am not saying it is entirely a bad idea, do not get me wrong. It just it is not enough to make it automatically secure. An application that does not filter input has to be modified to filter input, no way to get around it with magical Unix solutions. The OP is expecting a miracle that won't happen.
– Rui F Ribeiro
Aug 27 at 8:49

I am not saying it is entirely a bad idea, do not get me wrong. It just it is not enough to make it automatically secure. An application that does not filter input has to be modified to filter input, no way to get around it with magical Unix solutions. The OP is expecting a miracle that won't happen.
– Rui F Ribeiro
Aug 27 at 8:49

@danblack I think this would work except that I may not know all the user names upfront. Let me see if I can find a way to map all these users to few groups and sudo policy accordingly.
– Rajesh
Aug 27 at 9:21

@danblack I think this would work except that I may not know all the user names upfront. Let me see if I can find a way to map all these users to few groups and sudo policy accordingly.
– Rajesh
Aug 27 at 9:21

Scripts in the restricted folders are properly validating and sanitizing the input. The issue that I was trying to address is, avoiding injection to what is passed to system() call. With sudo conf changes, I can change the code from /bin/bash -r -c <name-of-restricted-script> <arg> to - /bin/sudo -u admin <path-to-restricted-script> <arg>. Even if some one inject a command/script, it would either be executed with the original user permission or won't execute at all depending on permission of that script/command.
– Rajesh
Aug 27 at 9:29

Scripts in the restricted folders are properly validating and sanitizing the input. The issue that I was trying to address is, avoiding injection to what is passed to system() call. With sudo conf changes, I can change the code from /bin/bash -r -c <name-of-restricted-script> <arg> to - /bin/sudo -u admin <path-to-restricted-script> <arg>. Even if some one inject a command/script, it would either be executed with the original user permission or won't execute at all depending on permission of that script/command.
– Rajesh
Aug 27 at 9:29

 | 
show 1 more comment

up vote
2
down vote

Code injection requires the ability of the user to pass arbitrary strings as parameters to the system() call. This is pretty similar to SQL injections and should be avoided in a similar way: don't pass any user-defined string directly to the call:

• numerical parameters should be converted to integers, and then converted back to strings at the time of the call




• parameters which belong to a fixed dictionary should be converted to "enum" values or similar, then back to strings at the time of the call




• free text input should be restricted to inoffensive character set where possible (e.g. [a-zA-Z0-9]*). Where problematic characters (including space) are required, proper escaping should be applied (that is, a b should be replaced by a b, etc.)

share|improve this answer

answered Aug 27 at 9:49

Dmitry Grigoryev

4,937644

• 
  
  
  

  
  2
  

  
  
  
  
  
  

  
  backslash should be avoided for quoting as its encoding is part of some characters in some locale. Only single quotes are relatively safe. Note system() runs sh and the OP seems to want to run bash on top of that, so there may need to be two levels of quoting. Some sh implementations drop privileges when run setuid, so it may not work anyway. In any case, the environment would have to be sanitized as well.
  – Stéphane Chazelas
  Aug 27 at 10:46
  
  

  
  
  
  


• 
  
  
  

  
  

  
  
  
  
  
  

  
  DOn't forget to also run these checks on environment variables, and check all of them, not only PATH, also add a whitelist
  – Ferrybig
  Aug 27 at 10:58
  

  
  
  
  


• 
  
  
  

  
  1
  

  
  
  
  
  
  

  
  @Ferrybig, the only reasonable approach for the environment is to start with an empty one, and white list the ones that have harmless names and values and that may be needed like sudo does. Even then, there's harm that can be done with all the LC* ones or TZ for instance.
  – Stéphane Chazelas
  Aug 27 at 11:03
  

  
  
  
  

add a comment | 

up vote
2
down vote

Code injection requires the ability of the user to pass arbitrary strings as parameters to the system() call. This is pretty similar to SQL injections and should be avoided in a similar way: don't pass any user-defined string directly to the call:

• numerical parameters should be converted to integers, and then converted back to strings at the time of the call




• parameters which belong to a fixed dictionary should be converted to "enum" values or similar, then back to strings at the time of the call




• free text input should be restricted to inoffensive character set where possible (e.g. [a-zA-Z0-9]*). Where problematic characters (including space) are required, proper escaping should be applied (that is, a b should be replaced by a b, etc.)

share|improve this answer

answered Aug 27 at 9:49

Dmitry Grigoryev

4,937644

• 
  
  
  

  
  2
  

  
  
  
  
  
  

  
  backslash should be avoided for quoting as its encoding is part of some characters in some locale. Only single quotes are relatively safe. Note system() runs sh and the OP seems to want to run bash on top of that, so there may need to be two levels of quoting. Some sh implementations drop privileges when run setuid, so it may not work anyway. In any case, the environment would have to be sanitized as well.
  – Stéphane Chazelas
  Aug 27 at 10:46
  
  

  
  
  
  


• 
  
  
  

  
  

  
  
  
  
  
  

  
  DOn't forget to also run these checks on environment variables, and check all of them, not only PATH, also add a whitelist
  – Ferrybig
  Aug 27 at 10:58
  

  
  
  
  


• 
  
  
  

  
  1
  

  
  
  
  
  
  

  
  @Ferrybig, the only reasonable approach for the environment is to start with an empty one, and white list the ones that have harmless names and values and that may be needed like sudo does. Even then, there's harm that can be done with all the LC* ones or TZ for instance.
  – Stéphane Chazelas
  Aug 27 at 11:03
  

  
  
  
  

add a comment | 

up vote
2
down vote

up vote
2
down vote

Code injection requires the ability of the user to pass arbitrary strings as parameters to the system() call. This is pretty similar to SQL injections and should be avoided in a similar way: don't pass any user-defined string directly to the call:

• numerical parameters should be converted to integers, and then converted back to strings at the time of the call




• parameters which belong to a fixed dictionary should be converted to "enum" values or similar, then back to strings at the time of the call




• free text input should be restricted to inoffensive character set where possible (e.g. [a-zA-Z0-9]*). Where problematic characters (including space) are required, proper escaping should be applied (that is, a b should be replaced by a b, etc.)

share|improve this answer

answered Aug 27 at 9:49

Dmitry Grigoryev

4,937644

Code injection requires the ability of the user to pass arbitrary strings as parameters to the system() call. This is pretty similar to SQL injections and should be avoided in a similar way: don't pass any user-defined string directly to the call:

• numerical parameters should be converted to integers, and then converted back to strings at the time of the call




• parameters which belong to a fixed dictionary should be converted to "enum" values or similar, then back to strings at the time of the call




• free text input should be restricted to inoffensive character set where possible (e.g. [a-zA-Z0-9]*). Where problematic characters (including space) are required, proper escaping should be applied (that is, a b should be replaced by a b, etc.)

share|improve this answer

answered Aug 27 at 9:49

Dmitry Grigoryev

4,937644

share|improve this answer

share|improve this answer

answered Aug 27 at 9:49

Dmitry Grigoryev

4,937644

answered Aug 27 at 9:49

Dmitry Grigoryev

4,937644

answered Aug 27 at 9:49

Dmitry Grigoryev

4,937644

4,937644

• 
  
  
  

  
  2
  

  
  
  
  
  
  

  
  backslash should be avoided for quoting as its encoding is part of some characters in some locale. Only single quotes are relatively safe. Note system() runs sh and the OP seems to want to run bash on top of that, so there may need to be two levels of quoting. Some sh implementations drop privileges when run setuid, so it may not work anyway. In any case, the environment would have to be sanitized as well.
  – Stéphane Chazelas
  Aug 27 at 10:46
  
  

  
  
  
  


• 
  
  
  

  
  

  
  
  
  
  
  

  
  DOn't forget to also run these checks on environment variables, and check all of them, not only PATH, also add a whitelist
  – Ferrybig
  Aug 27 at 10:58
  

  
  
  
  


• 
  
  
  

  
  1
  

  
  
  
  
  
  

  
  @Ferrybig, the only reasonable approach for the environment is to start with an empty one, and white list the ones that have harmless names and values and that may be needed like sudo does. Even then, there's harm that can be done with all the LC* ones or TZ for instance.
  – Stéphane Chazelas
  Aug 27 at 11:03
  

  
  
  
  

add a comment | 

• 
  
  
  

  
  2
  

  
  
  
  
  
  

  
  backslash should be avoided for quoting as its encoding is part of some characters in some locale. Only single quotes are relatively safe. Note system() runs sh and the OP seems to want to run bash on top of that, so there may need to be two levels of quoting. Some sh implementations drop privileges when run setuid, so it may not work anyway. In any case, the environment would have to be sanitized as well.
  – Stéphane Chazelas
  Aug 27 at 10:46
  
  

  
  
  
  


• 
  
  
  

  
  

  
  
  
  
  
  

  
  DOn't forget to also run these checks on environment variables, and check all of them, not only PATH, also add a whitelist
  – Ferrybig
  Aug 27 at 10:58
  

  
  
  
  


• 
  
  
  

  
  1
  

  
  
  
  
  
  

  
  @Ferrybig, the only reasonable approach for the environment is to start with an empty one, and white list the ones that have harmless names and values and that may be needed like sudo does. Even then, there's harm that can be done with all the LC* ones or TZ for instance.
  – Stéphane Chazelas
  Aug 27 at 11:03
  

  
  
  
  

2

2

backslash should be avoided for quoting as its encoding is part of some characters in some locale. Only single quotes are relatively safe. Note system() runs sh and the OP seems to want to run bash on top of that, so there may need to be two levels of quoting. Some sh implementations drop privileges when run setuid, so it may not work anyway. In any case, the environment would have to be sanitized as well.
– Stéphane Chazelas
Aug 27 at 10:46

backslash should be avoided for quoting as its encoding is part of some characters in some locale. Only single quotes are relatively safe. Note system() runs sh and the OP seems to want to run bash on top of that, so there may need to be two levels of quoting. Some sh implementations drop privileges when run setuid, so it may not work anyway. In any case, the environment would have to be sanitized as well.
– Stéphane Chazelas
Aug 27 at 10:46

DOn't forget to also run these checks on environment variables, and check all of them, not only PATH, also add a whitelist
– Ferrybig
Aug 27 at 10:58

DOn't forget to also run these checks on environment variables, and check all of them, not only PATH, also add a whitelist
– Ferrybig
Aug 27 at 10:58

1

1

@Ferrybig, the only reasonable approach for the environment is to start with an empty one, and white list the ones that have harmless names and values and that may be needed like sudo does. Even then, there's harm that can be done with all the LC* ones or TZ for instance.
– Stéphane Chazelas
Aug 27 at 11:03

@Ferrybig, the only reasonable approach for the environment is to start with an empty one, and white list the ones that have harmless names and values and that may be needed like sudo does. Even then, there's harm that can be done with all the LC* ones or TZ for instance.
– Stéphane Chazelas
Aug 27 at 11:03

add a comment |