exec_in_shell doesn't work about zfs-toolbox HOT 11 CLOSED

I assume your problem is on the 'zpool import' call. That's where I'm seeing the problem. I don't think you're doing anything wrong. I think, after a couple of experiments, that 'zpool import' returns 1 if there are no pools to import. I'm looking into it.

from zfs-toolbox.

Yes, it was on the 'zpool import'. I believe I was also seeing the zpool usage message, though, which makes me think it is more than just an unexpected return code.

from zfs-toolbox.

Yes, I saw that too. For some reason, it doesn't seem to like the split array. I think I may have worked it all out this morning, although I didn't have time to test and what-not, so I'll fix this tonight.

from zfs-toolbox.

Fix: Pass absolute paths for 'zpool' and 'zfs' commands, use shell=False.

from zfs-toolbox.

It seems that this has the unfortunate consequence that now the "|" command does not work. I'm pretty sure you can pass "|" into check_output only if shell=False. I'm getting this failure on the zfs send ... | zfs receive ...

from zfs-toolbox.

I assume you mean "you can pipe only if shell=True"?

I suspected this would happen. Not running in the shell anymore means we can't just pipe as if we were in the shell. Probably need shell=True for these commands that want to pipe, or pipe through python (which is going to be kind of gross, but probably the better solution in this case).

This feels like a separate bug to me.

from zfs-toolbox.

Attempted a fix in e76243f. Untested.

from zfs-toolbox.

I haven't tested yet either. I'll look at it tonight.

There's a separate problem with shell=True, by the way. Namely, if any part of the command is untrusted, then we have a vulnerability, especially since this thing is being run as root. If we don't trust the input (and I don't think we should trust the pool names, for example), then the input can execute arbitrary code in the shell, for example by using backticks. In summary, I don't think using shell=True is a good idea anyway.

from zfs-toolbox.

Correct. If the configuration file is writable by an unprivileged user,
this results in a local privilege escalation vulnerability.

On Wed, Jul 11, 2012 at 10:06 AM, Joel Weinberger <
[email protected]

wrote:

I haven't tested yet either. I'll look at it tonight.

There's a separate problem with shell=True, by the way. Namely, if any
part of the command is untrusted, then we have a vulnerability, especially
since this thing is being run as root. If we don't trust the input (and I
don't think we should trust the pool names, for example), then the input
can execute arbitrary code in the shell, for example by using backticks. In
summary, I don't think using shell=True is a good idea anyway.

---

Reply to this email directly or view it on GitHub:
https://github.com/mfinifter/zfs-auto-backup/issues/3#issuecomment-6912912

from zfs-toolbox.

Succinctly said!

On Wed, Jul 11, 2012 at 10:09 AM, Matthew Finifter <
[email protected]

wrote:

Correct. If the configuration file is writable by an unprivileged user,
this results in a local privilege escalation vulnerability.

On Wed, Jul 11, 2012 at 10:06 AM, Joel Weinberger <
[email protected]

wrote:

I haven't tested yet either. I'll look at it tonight.

There's a separate problem with shell=True, by the way. Namely, if any
part of the command is untrusted, then we have a vulnerability,
especially
since this thing is being run as root. If we don't trust the input (and I
don't think we should trust the pool names, for example), then the input
can execute arbitrary code in the shell, for example by using backticks.
In
summary, I don't think using shell=True is a good idea anyway.

---

Reply to this email directly or view it on GitHub:

https://github.com/mfinifter/zfs-auto-backup/issues/3#issuecomment-6912912

---

Reply to this email directly or view it on GitHub:
https://github.com/mfinifter/zfs-auto-backup/issues/3#issuecomment-6912982

from zfs-toolbox.

After a bit of testing, this seems to be fixed. I think we can go ahead and call this one solved!

from zfs-toolbox.