-
-
Notifications
You must be signed in to change notification settings - Fork 925
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Kernel#system fails to execute if SecurityManager denies access to a $PATH entry even if it permits a later one #3983
Comments
you should try doing so - catching |
@trejkaz Yes, your theory sounds right. We do have PATH-searching logic, and if it gets a security error on one entry it should proceed to the next. |
@trejkaz Can you try the following patch on your system, please? diff --git a/core/src/main/java/org/jruby/util/ShellLauncher.java b/core/src/main/java/org/jruby/util/ShellLauncher.java
index 528dab2..655027d 100644
--- a/core/src/main/java/org/jruby/util/ShellLauncher.java
+++ b/core/src/main/java/org/jruby/util/ShellLauncher.java
@@ -398,9 +398,14 @@ public class ShellLauncher {
// NOTE: Jruby's handling of tildes is more complete than
// MRI's, which can't handle user names after the tilde
// when searching the executable path
- pathFile = isValidFile(runtime, fdir, fname, isExec);
- if (pathFile != null) {
- break;
+ try {
+ pathFile = isValidFile(runtime, fdir, fname, isExec);
+ if (pathFile != null) {
+ break;
+ }
+ } catch (SecurityException se) {
+ // Security prevented accessing this PATH entry, proceed to next
+ continue;
}
}
} else { |
@trejkaz We'd still like to include this in 9.1.3.0 if it works properly for you. Note to others: we'll need to look at adding a bit of security magic to the native process logic. |
We should add a test for this as well...something that has to traverse a directory with no permissions to find one that does before executing the command. |
Argh, I knew I forgot to do something. I will try and get onto this tomorrow. |
@trejkaz ping :-) |
I'm trying. I guess this is the typical experience when I try to build something based on Maven, but it's like no version will work. :(
What I hate the most about Maven is that when it fails, it gives absolutely no hint to allow me to go and fix it. "Compile failure" doesn't really cut it and they should feel bad for not at least showing what the failure was. I love how they even say "After correcting the problems", as if they have provided enough information to do so.
|
OK, I finally got to the bottom of this. JNR has some magic it's doing with OpenFlags which seemingly parses the class file or something, perhaps as an alternative to using reflection(?). That trips up the security manager, and then URLClassPath masks the error and returns null, which was very hard to diagnose until I got into a debugger. It seems a little illogical that findResource(String) would return null when getClass(String) for the exact same class was able to resolve the class, though. Maybe they should have found a way to do it without this magic... Anyway, this only tripped up the security manager because I was loading the JRuby jar from a file. The actual security policy is that reading from any jar on the classpath is OK, so I temporarily added the JRuby jar to the policy while testing. Now it goes through, I see it catch the exception and it then finds I do get some warning on stderr:
I'm not calling |
Ok, that's good to hear.
Yeah that's kinda goofy. We should find that warning and fix it so it actually makes sense :-) I'll call this fixed and at least patch up the wording of that warning. |
Oh sorry, you meant it's fixed with the patch I provided above, yes? I had not yet committed that to master. |
This patch allows the PATH searching logic for non-native process launching to proceed past paths which trigger security exceptions, such as any path not explicitly permitted for reading by JVM security policies. Fixes #3983.
I'd like to see tests for this, but I'm not sure what they'd look like. |
Yeah, I meant after applying the patch myself and running that version, it's fixed. :) |
Environment
JRuby 9.0.5.0 core + stdlib (stopped using complete because it bundled extra things which clashed with other dependencies).
OS here is Mac OS X 10.11.5.
We run tests with the security manager enabled, mostly to stop people writing files all over the computer.
One such test I tried to write was when someone reported that the
system
function didn't work.Expected Behavior
I expect this to pass:
Our security policy does allow executing java:
Actual Behavior
ShellLauncher
gets to the catch block here:Exception is:
Stack for that:
So it seems like
ShellLauncher
has some logic where it programmatically searches$PATH
for the executable.$HOME/bin
is the first entry in my path and the security policy does not allow reading from there, so anAccessControlException
is thrown. This then doesn't get caught, and for some reasonShellLauncher
immediately aborts. Perhaps what it should be doing is catching that and trying the next location.The text was updated successfully, but these errors were encountered: