Replies: 36 comments
-
|
First off, I would suggest looking at the Junit test example, here: https://github.com/ESAPI/esapi-java-legacy/blob/develop/src/test/java/org/owasp/esapi/reference/ExecutorTest.java#L245-L279 (You shouldn't need the stuff on lines 259-265 since you presumably are not running this from JUnit so you don't need to override the ESAPI.properties configuration.) However, what seems odd to me is that the places where you are explicitly including the double-quote. I don't think that is needed. I think ESAPI will add it for you when you call the executeSystemCommand method. (At least, I don't see that in the JUnit example.) Thus, instead of: params.add("\"chown");and params.add("/var/test/123.sh\"");try: params.add("chown");and params.add("/var/test/123.sh");respectively. That's more in line with the JUnit example. Let us know if it works or not. |
Beta Was this translation helpful? Give feedback.
-
|
Hi, thanks for the quick response. I have tried your method and it returns me the error
|
Beta Was this translation helpful? Give feedback.
-
|
@raine93 - That looks like you missed syntax to chown. |
Beta Was this translation helpful? Give feedback.
-
|
I change my code to this params.add("-c"); It is still the same error chown: missing operand If I run the command (/usr/bin/bash -c chown user1 /var/test/123.sh) in the terminal, it will have the same error. Looks like it did not add the double quotes in the executeSystemCommand
|
Beta Was this translation helpful? Give feedback.
-
|
https://linux.die.net/man/1/chown EG:
From what I'm gathering, your command might look something like: to replicate that, I believe you would create the code much like below Please note that you'll need to substitute a valid group for user1 on your system. |
Beta Was this translation helpful? Give feedback.
-
|
No, the command is correct. in my code: Executor executor = ESAPI.executor(); Codec codec = new UnixCodec(); File workdir = new File( "/var"); params.add("-c"); ExecuteResult result = executor.executeSystemCommand(binSh, params, workdir, codec, true, false); I am trying to execute command: -c is the option for bash to execute command string. I have tested the command in terminal and it works. However, without the double quotes, it will have the same error of missing operand
|
Beta Was this translation helpful? Give feedback.
-
|
What additional output do you see when you redirect the errorstream? |
Beta Was this translation helpful? Give feedback.
-
|
Is the same error: chown: missing operand
|
Beta Was this translation helpful? Give feedback.
-
|
Your very first command should work if you use “chmod” instead of “chown” Chown doesn’t take a permissions argument like you’re passing. |
Beta Was this translation helpful? Give feedback.
-
|
What Linux distro are you using? chown and chmod each might be in /usr/sbin also worth checking to see what user your Java process is running as, if it’s say, tomcat:tomcat it won’t have access to those commands. |
Beta Was this translation helpful? Give feedback.
-
|
Yes, i realized I made a mistake then I change to chown but the error is still happening because there is no double quotes if I do not specify and if I specify, it will escape my quotes
|
Beta Was this translation helpful? Give feedback.
-
|
I am using oracle 8 with latest esapi release version. The error happening now is missing operand instead of command not found. I am thinking executeSystemCommand does not support executing command string with bash -c
|
Beta Was this translation helpful? Give feedback.
-
|
Apologies, Oracle 8 isn’t an operating system unless I’m missing something, Oracle 8 ought to be running on top of something else.
I’m willing to try testing but it’s a crapshoot if I don’t use the same OS. RHEL does some different things from Debian.
…On Nov 22, 2024 at 22:32 -0700, PQ C ***@***.***>, wrote:
I am using oracle 8 with latest esapi release version. The error happening now is missing operand instead of command not found. I am thinking executeSystemCommand does not support executing command string with bash -c
> What Linux distro are you using?
> chown and chmod each might be in /usr/sbin
> also worth checking to see what user your Java process is running as, if it’s say, tomcat:tomcat it won’t have access to those commands.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you commented.Message ID: ***@***.***>
|
Beta Was this translation helpful? Give feedback.
-
|
Suggestion : run your code in a debugger and set a breakpoint in
DefaultExecutor.executeSystemCommand in the variant you are using, and
single step through it. Or maybe set the log level to DEBUG. I'm heading to
bed, but will try to look at it tomorrow.
…-kevin
On Sat, Nov 23, 2024, 12:32 AM PQ C ***@***.***> wrote:
I am using oracle 8 with latest esapi release version. The error happening
now is missing operand instead of command not found. I am thinking
executeSystemCommand does not support executing command string with bash -c
What Linux distro are you using?
chown and chmod each might be in /usr/sbin
also worth checking to see what user your Java process is running as, if
it’s say, tomcat:tomcat it won’t have access to those commands.
—
Reply to this email directly, view it on GitHub
<#858 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAO6PG7VZNZ5DMSM5GJAODL2CAHQRAVCNFSM6AAAAABSF5OIPCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDIOJVGMZDKNBUGI>
.
You are receiving this because you commented.Message ID:
***@***.***>
|
Beta Was this translation helpful? Give feedback.
-
|
I am using Oracle Linux 8 and it is built on RHEL
|
Beta Was this translation helpful? Give feedback.
-
|
If I use like this in ProcessBuilder: params.add("/usr/bin/bash"); this one will work |
Beta Was this translation helpful? Give feedback.
-
|
So given the log, its clear that output from the command is getting passed to the command line, and THAT command is failing with a nonzero status. So you need to get a sample of what's actually getting run on the command line. Once we're there we can diagnose what's malformed in the command.
|
Beta Was this translation helpful? Give feedback.
-
|
From the logs , the command it is running is this /usr/bin/bash -c chown user1 \/var\/test\/123\.sh
|
Beta Was this translation helpful? Give feedback.
-
|
is there a way to set the immune characters for UnixCodec and pass to executeSystemCommand? |
Beta Was this translation helpful? Give feedback.
-
|
@raine93 - Before we go any further down this rabbit hole, let me ask you a really important question. I really should have asked you this from the start. My bad for failing to do so. My question is, once you get this working, what is your intended use case for it? That is, what problem are you trying to solve? (Or more precisely, why are you allowing the shell and seemingly any arbitrary command string?) Because the fact that you are using /bin/sh -c "some arbitrary command string and args ..."is a bit bothersome to me. There is a reason that the Executor.ApprovedExecutables property is required to be set (as a comma separated like of commands with full path names IIRC) to the executables you are permitting. That allows you to somewhat restrict which commands that can be run. I just don't to want to give you the impression that the As soon as you allow a shell and the '-c' argument, then at that point, you are allowing pretty much any arbitrary command string. And unless your goal it to more or less accidentally provide a reverse shell to attackers, this is NOT going to be made safe. For instance, do you really want to allow someone to execute "rm -fr ."? Probably not. (Granted, our own examples in ESAPI.properties for the Executor.ApprovedExecutables property shows So I just want you to be made 100% aware of that. If your intent was to limit your users to only executing certain commands (say, for example, And then instead of: params.add("/usr/bin/bash");
params.add("-c");
params.add("chown user1 \/var\/test\/123.sh");you would do something like: (I don't think you need to quote the with '' for the file path name.) That said, your original example was: Codec codec = new UnixCodec();
List params = new ArrayList();
File workdir = new File( "/var");
params.add("-c");
params.add("chown");
params.add("user1");
params.add("/var/test/123.sh");
ExecuteResult result = executor.executeSystemCommand(binSh, params, workdir, codec, true, false);Instead, if you really want to shoot yourself in the foot, I think this ought to be: Codec codec = new UnixCodec();
List params = new ArrayList();
File workdir = new File( "/var");
params.add("-c");
params.add("chown user1 /var/test/123.sh"); // Or perhaps: params.add("'chown user1 /var/test/123.sh'") if the former fails
ExecuteResult result = executor.executeSystemCommand(binSh, params, workdir, codec, true, false);because as per the manual page, the syntax for sh is "-c command_string" which means if you have multiple arguments you need to enclose them in quotes together for the to be processed as a single string. If you like foot cannons, give that a try. |
Beta Was this translation helpful? Give feedback.
-
|
@kwwall You are right. I should not be using the arbitrary command. I have some commands need to use such as chmod, chown and systemctl. But my commands require to start with sudo (sudo systemctl stop chronyd). How should I do with this? Should I add sudo in approved executables and have a allowed list of commands? I also see that the UnixCodec will escape my spacing (chown\ user1\ ....) which causes error in ProcessBuilder. |
Beta Was this translation helpful? Give feedback.
-
IIRC you can do that IFF you create your own Codec. However, making the forward slash an immune char directly makes that method vulnerable to the exact attack we're trying to prevent here. So let me start out by saying this: The original intent of this library was to provide emergency security functionality for java applications that had just been breached. Functions like this one are really only intended as a stopgap for patching application functionality when no other easier alternative is available, for example, with SQLi, you know you need to rewrite your query to use a Parameterized query, but you need to just fix this shit right now because you don't have time to rewrite your entire application's SQL interface. For what you're attempting to do here:
You should actually be using the Java nio API to accomplish. But as I started cruising documentation in nio... the thought struck me, what is it exactly that we're trying to accomplish here? What's the bigger story? As a general rule I always avoid ever having to pass commands to a command line. There's a bad smell here... chown usually implies sudo or root, and your java web application should never be running with elevated permissions. (This is why I originally asked what the outputs were to The whole point of the UnixCodec, is to ensure that when you're passing data to a Unix command line, that the output can only be treated as data. That's the exact purpose. Your command is being "broken" because that's the intended consequence. For the executor failing, let's ask the question first, why do you think you need to use it? Are you allowing the user to specify commands to run on the back end? If so, this is an antipattern as well. If the answer is no as I'm guessing, you don't need to use the executor at all, because you should have a mapping between web application user and unix system user if this is the case, and you can read the user's input and validate against it, but opt only to use actual input you control yourself. i.e., get all the users for your target group and hold that in memory. Your command string should be hardcoded, something like "chown %s /var/test/%s.sh" so something like In this case, you don't need to worry at all about escaping the first For the second, are we taking the user's input for the filename? I would avoid this and use a naming convention that the user cannot control. If you really cannot avoid this, NOW you can consider using the UnixCodec to escape JUST THE FILE NAME. Do NOT allow users to control *NIX paths on the command line, this is just a terrible practice. NOW you're in a position to actually invalidate an attack. The UnixCodec (which I believe is used when you pass to executor) is supposed to stop a parameter to a UNIX command from being changed to something like " && rm -rf ../../../ " or some other dastardly deed. You're breaking the intent of security by allowing forward slashes in your input, i.e. you're trying to allow an unsafe practice. Reduce the threat surface by eliminating as much user control as possible, substituting values that you have a guarantee are safe. Part of your problem is that the intended use of the executor and the UnixCodec is to escape only the smallest sequence possible, NOT a full path. When dealing with system commands, always hardcode as much as you can, in this particular case, hardcoding is fine because it means safety. Given that there's no equivalent to a PreparedStatement when dealing with system commands, excessive safety is the best thing we have. So control the structure of the end command, simplify the user input variables to make the cognitive overhead easier to manage (I.E., only validate/process against what gets substituted into the And also, if you have to take user input for the username (please don't) you'll want to escape that for Unix as well. |
Beta Was this translation helpful? Give feedback.
-
|
@kwwall or @jeremiahjstacey I may have goofed on a detail here, but as I studied the problem, this isn't a bug with ESAPI, it's an example of it being used for something it wasn't designed for. i.e., we have improper documentation on how to use this for *nix escaping. |
Beta Was this translation helpful? Give feedback.
-
|
I agree. Let's move this 'issue' to Discussions and we can continue the
dialog there.
…On Wed, Dec 4, 2024, 10:37 AM Matt Seil ***@***.***> wrote:
@kwwall <https://github.com/kwwall> or @jeremiahjstacey
<https://github.com/jeremiahjstacey> I may have goofed on a detail here,
but as I studied the problem, this isn't a bug with ESAPI, it's an example
of it being used for something it wasn't designed for. i.e., we have
improper documentation on how to use this for *nix escaping.
—
Reply to this email directly, view it on GitHub
<#858 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAO6PG4OHMKD5MMIKWDWLT32D4OTFAVCNFSM6AAAAABSF5OIPCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDKMJXHAYDGNZVGY>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Beta Was this translation helpful? Give feedback.
-
|
@xeno6696 the commands chown, chmod are hardcoded, not using user input. If my web app fail to write to a file, I will execute it to change the permissions and ownership if necessary. However, I need to start the command with sudo to allow to change. I also use grep command to search for multiple keywords in list of files. Each keyword I intend to wrap it with single quote. Keyword(s) is user input. I disallow user to key in single and double quotes |
Beta Was this translation helpful? Give feedback.
-
|
So you need to hardcode more than that, is my point: in the most defensive case, validate both %s arguments using SafeString as I indicated then you can escape each %s using unix codec when binding parameters. Kevin gave you great advice above: |
Beta Was this translation helpful? Give feedback.
-
|
Okay, finally set up a laptop with linux to test this out. I think this is failing due to your linux shell. Using the following example program: I set a breakpoint on line 25 (while loop) and then checked the ps command output with
After capturing the literal command being run (this is failing with an exit status of 1 on my machine) I then try to run that on the command line:
I think this perfectly reproduces your behavior. This is using straight processbuilder, and not esapi at all, so I think, you need to get your command to work HERE first. (I'm able to actually engage in some troubleshooting now and not just talking out of my head.) Replying to your "built my own encoder with the immune char" what you have done here is built a vulnerable fork of ESAPI. Which is fine, but if you go that path, I'm officially done offering any help or support. If people want to cut their own hands off, that's their business. |
Beta Was this translation helpful? Give feedback.
-
|
Confirmed, the command works if you treat the entire command as a string:
This makes sense when you consider that when you're trying to run a command using /usr/bin/sh you're really creating an instance to be parsed, so what ends up happening is
I still think we're not approaching this correctly overall. You should be able to change permissions/groups just using java nio. Why are you storing files on the local filesystem? Why not use a database blob? |
Beta Was this translation helpful? Give feedback.
-
|
@kwwall Doing some more poking. Created the unit test: This is a similar issue as we've encountered in the past where users were messing with canonicalize and the URL function. ESAPI's input/output encoding are expressly designed to avoid multiple encoding contexts. The reason why this guy's stuff is failing is that he's doing a use case that this library was never intended to deal with, namely the use of The method documentation of DefaultExecutor.executeSystemCommand() very clearly makes the case that the idea behind the method is to execute naked commands, and I believe Which is also technically multiple encoding.
You have to encode the command string to shell1: |
Beta Was this translation helpful? Give feedback.
-
|
@raine93 - I am converting this issue to a Discussion. Note that doing so will lock this issue so no further comments can be made, but you can continue comments under Discussions. |
Beta Was this translation helpful? Give feedback.
-
I am trying to execute a Linux command using executeSystemCommand but it unable to execute
Command:
/bin/sh -c "chown 0777 /var/test/123.sh"
tried to follow this solution but it will just give error 0777: "chown: command not found
https://stackoverflow.com/questions/71204716/how-should-esapi-executesystemcommand-sanitise-the-file-path-properly-to-satisfy
I put the full path of chown, it will also give the error 0777: "/usr/bin/chown: No such file or directory
Code:
Executor executor = ESAPI.executor();
File binSh = new File("/bin/sh").getCanonicalFile();
Codec codec = new UnixCodec();
List params = new ArrayList();
File workdir = new File( "/var");
params.add("-c");
params.add("\"chown");
params.add("0777");
params.add("/var/test/123.sh\"");
ExecuteResult result = executor.executeSystemCommand(binSh, params, workdir, codec, true, false);
Beta Was this translation helpful? Give feedback.
All reactions