Avoid command injection with system() api [closed]
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
edited Aug 27 '18 at 5:05
Archemar
20.2k93772
asked Aug 27 '18 at 4:50
RajeshRajesh
283
closed as unclear what you're asking by Rui F Ribeiro, muru, schily, Romeo Ninov, andcoz Aug 27 '18 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.
add a comment |
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
edited Aug 27 '18 at 5:05
Archemar
20.2k93772
asked Aug 27 '18 at 4:50
RajeshRajesh
283
closed as unclear what you're asking by Rui F Ribeiro, muru, schily, Romeo Ninov, andcoz Aug 27 '18 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 '18 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 '18 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 '18 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 '18 at 11:13
add a comment |
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
edited Aug 27 '18 at 5:05
Archemar
20.2k93772
asked Aug 27 '18 at 4:50
RajeshRajesh
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
linux c
edited Aug 27 '18 at 5:05
Archemar
20.2k93772
asked Aug 27 '18 at 4:50
RajeshRajesh
283
edited Aug 27 '18 at 5:05
Archemar
20.2k93772
asked Aug 27 '18 at 4:50
RajeshRajesh
283
edited Aug 27 '18 at 5:05
Archemar
20.2k93772
edited Aug 27 '18 at 5:05
Archemar
20.2k93772
edited Aug 27 '18 at 5:05
Archemar
20.2k93772
20.2k93772
asked Aug 27 '18 at 4:50
RajeshRajesh
283
asked Aug 27 '18 at 4:50
RajeshRajesh
283
asked Aug 27 '18 at 4:50
RajeshRajesh
283
283
closed as unclear what you're asking by Rui F Ribeiro, muru, schily, Romeo Ninov, andcoz Aug 27 '18 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 '18 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 '18 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 '18 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 '18 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 '18 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 '18 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 '18 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 '18 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 '18 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 '18 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 '18 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 '18 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 '18 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 '18 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 '18 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 '18 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 '18 at 11:13
add a comment |
2 Answers
2
active
oldest
votes
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.
answered Aug 27 '18 at 5:44
danblackdanblack
3666
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 '18 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 callssystemand is suid. I agree mistakes still can be made. I'll reword so it doesn't sound so flippant.
– danblack
Aug 27 '18 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 '18 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 '18 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 tosystem()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 '18 at 9:29
|
show 1 more comment
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 bshould be replaced bya b, etc.)
answered Aug 27 '18 at 9:49
Dmitry GrigoryevDmitry Grigoryev
5,105945
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 '18 at 10:46
DOn't forget to also run these checks on environment variables, and check all of them, not onlyPATH, also add a whitelist
– Ferrybig
Aug 27 '18 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 theLC*ones orTZfor instance.
– Stéphane Chazelas
Aug 27 '18 at 11:03
add a comment |
2 Answers
2
active
oldest
votes
2 Answers
2
active
oldest
votes
active
oldest
votes
active
oldest
votes
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.
answered Aug 27 '18 at 5:44
danblackdanblack
3666
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 '18 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 callssystemand is suid. I agree mistakes still can be made. I'll reword so it doesn't sound so flippant.
– danblack
Aug 27 '18 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 '18 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 '18 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 tosystem()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 '18 at 9:29
|
show 1 more comment
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.
answered Aug 27 '18 at 5:44
danblackdanblack
3666
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 '18 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 callssystemand is suid. I agree mistakes still can be made. I'll reword so it doesn't sound so flippant.
– danblack
Aug 27 '18 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 '18 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 '18 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 tosystem()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 '18 at 9:29
|
show 1 more comment
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.
answered Aug 27 '18 at 5:44
danblackdanblack
3666
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.
answered Aug 27 '18 at 5:44
danblackdanblack
3666
edited Aug 27 '18 at 8:52
answered Aug 27 '18 at 5:44
danblackdanblack
3666
answered Aug 27 '18 at 5:44
danblackdanblack
3666
answered Aug 27 '18 at 5:44
danblackdanblack
3666
3666
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 '18 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 callssystemand is suid. I agree mistakes still can be made. I'll reword so it doesn't sound so flippant.
– danblack
Aug 27 '18 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 '18 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 '18 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 tosystem()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 '18 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 '18 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 callssystemand is suid. I agree mistakes still can be made. I'll reword so it doesn't sound so flippant.
– danblack
Aug 27 '18 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 '18 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 '18 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 tosystem()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 '18 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 '18 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 '18 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 '18 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 '18 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 '18 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 '18 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 '18 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 '18 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 '18 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 '18 at 9:29
|
show 1 more comment
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 bshould be replaced bya b, etc.)
answered Aug 27 '18 at 9:49
Dmitry GrigoryevDmitry Grigoryev
5,105945
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 '18 at 10:46
DOn't forget to also run these checks on environment variables, and check all of them, not onlyPATH, also add a whitelist
– Ferrybig
Aug 27 '18 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 theLC*ones orTZfor instance.
– Stéphane Chazelas
Aug 27 '18 at 11:03
add a comment |
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 bshould be replaced bya b, etc.)
answered Aug 27 '18 at 9:49
Dmitry GrigoryevDmitry Grigoryev
5,105945
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 '18 at 10:46
DOn't forget to also run these checks on environment variables, and check all of them, not onlyPATH, also add a whitelist
– Ferrybig
Aug 27 '18 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 theLC*ones orTZfor instance.
– Stéphane Chazelas
Aug 27 '18 at 11:03
add a comment |
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 bshould be replaced bya b, etc.)
answered Aug 27 '18 at 9:49
Dmitry GrigoryevDmitry Grigoryev
5,105945
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 bshould be replaced bya b, etc.)
answered Aug 27 '18 at 9:49
Dmitry GrigoryevDmitry Grigoryev
5,105945
answered Aug 27 '18 at 9:49
Dmitry GrigoryevDmitry Grigoryev
5,105945
answered Aug 27 '18 at 9:49
Dmitry GrigoryevDmitry Grigoryev
5,105945
answered Aug 27 '18 at 9:49
Dmitry GrigoryevDmitry Grigoryev
5,105945
5,105945
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 '18 at 10:46
DOn't forget to also run these checks on environment variables, and check all of them, not onlyPATH, also add a whitelist
– Ferrybig
Aug 27 '18 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 theLC*ones orTZfor instance.
– Stéphane Chazelas
Aug 27 '18 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 '18 at 10:46
DOn't forget to also run these checks on environment variables, and check all of them, not onlyPATH, also add a whitelist
– Ferrybig
Aug 27 '18 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 theLC*ones orTZfor instance.
– Stéphane Chazelas
Aug 27 '18 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 '18 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 '18 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 '18 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 '18 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 '18 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 '18 at 11:03
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 '18 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 '18 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 '18 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 '18 at 11:13