Re: [sage-devel] What to do with the sys.path security patch?
On Fri, Mar 1, 2019 at 12:14 AM Michael Orlitzky wrote: > > On 2/28/19 10:53 AM, Jeroen Demeyer wrote: > > Since many years, we have patched the Python in Sage with a patch to > > mitigate some security issues related to sys.path. > > > > There are two issues here: > > 1) You should never be doing anything at a predictable world-writable > location. > > Do we do that in SageMath? If we do, we should fix it to use the > equivalent of "mktemp -d" instead. Then we'll be working in an empty > directory that we own, which no one could have predicted beforehand. > > 2) Having the current directory in sys.path is unsafe. > > This is orthogonal to the first issue in my opinion. Yeah, we should > talk upstream into fixing this, using Perl's example as a shame-stick. > But, I don't think we need to carry python patches to fix this: > > a) It's not a huge vulnerability. > > b) I would say that fixing it in every single application is the wrong > approach anyway. Really, you shouldn't be running an executable in > a directory that is writable by anyone but you; otherwise, one of > those people can replace your program with one of their choosing, > and escalate privileges. Grsecurity used to have patches for the > linux kernel that would prevent you from doing that. Obviously > it breaks compatibility all over the place, but is really the > right way to fix the problem, since the situation is always > exploitable. > > c) So long as we're not doing something dumb ourselves (i.e. #1 > above), I don't think it's our job to make sure that the user > doesn't shoot himself in the foot by doing something that upstream > python allows. (He could just use /usr/bin/python, after all.) +1 to everything Michael wrote--this expresses what I've been trying to say about this patch since I first discovered it, but more clearly worded than I think I have been in the past. I also agree with Jeroen that Python *should* be patched for this as well. I used to argue that it was pointless for many of the above reasons. But I've changed my mind and now think it's a harmless change for a little bit of extra peace of mind. Just, not ultimately that broadly applicable. To answer, > 1) You should never be doing anything at a predictable world-writable > location. > > Do we do that in SageMath? If we do, we should fix it to use the > equivalent of "mktemp -d" instead. Then we'll be working in an empty > directory that we own, which no one could have predicted beforehand. Generally no. Most temp files made by sagelib itself actually go into the user's DOT_SAGE directory, which is typically, by default, in the user's home directory. There *might* be some software (though I'm not sure) that's wrapped by sage that is not so good. I think in general we try where possible to get all of Sage dependencies to write temp files and the like into DOT_SAGE, but maybe there are some we still miss? That's an upstream issue though. -- You received this message because you are subscribed to the Google Groups "sage-devel" group. To unsubscribe from this group and stop receiving emails from it, send an email to sage-devel+unsubscr...@googlegroups.com. To post to this group, send email to sage-devel@googlegroups.com. Visit this group at https://groups.google.com/group/sage-devel. For more options, visit https://groups.google.com/d/optout.
Re: [sage-devel] What to do with the sys.path security patch?
On 2/28/19 10:53 AM, Jeroen Demeyer wrote: > Since many years, we have patched the Python in Sage with a patch to > mitigate some security issues related to sys.path. > There are two issues here: 1) You should never be doing anything at a predictable world-writable location. Do we do that in SageMath? If we do, we should fix it to use the equivalent of "mktemp -d" instead. Then we'll be working in an empty directory that we own, which no one could have predicted beforehand. 2) Having the current directory in sys.path is unsafe. This is orthogonal to the first issue in my opinion. Yeah, we should talk upstream into fixing this, using Perl's example as a shame-stick. But, I don't think we need to carry python patches to fix this: a) It's not a huge vulnerability. b) I would say that fixing it in every single application is the wrong approach anyway. Really, you shouldn't be running an executable in a directory that is writable by anyone but you; otherwise, one of those people can replace your program with one of their choosing, and escalate privileges. Grsecurity used to have patches for the linux kernel that would prevent you from doing that. Obviously it breaks compatibility all over the place, but is really the right way to fix the problem, since the situation is always exploitable. c) So long as we're not doing something dumb ourselves (i.e. #1 above), I don't think it's our job to make sure that the user doesn't shoot himself in the foot by doing something that upstream python allows. (He could just use /usr/bin/python, after all.) -- You received this message because you are subscribed to the Google Groups "sage-devel" group. To unsubscribe from this group and stop receiving emails from it, send an email to sage-devel+unsubscr...@googlegroups.com. To post to this group, send email to sage-devel@googlegroups.com. Visit this group at https://groups.google.com/group/sage-devel. For more options, visit https://groups.google.com/d/optout.
Re: [sage-devel] What to do with the sys.path security patch?
On Thursday, February 28, 2019 at 9:16:19 AM UTC-8, E. Madison Bray wrote: > > > I suggest a middle ground: I don't believe this behavior should be > tested in Sage's test suite, because this is a question about the > Python interpreter's behavior, not Sage. The possibility of running > Sage on a Python interpreter provided by the system, which does not > have this patch, means the behavior here cannot be controlled for. > > So I think if you want to include this patch in Sage, the patch should > include patches to Python's own test suite (as would be needed in any > case to have any chance of getting it upstream) so that it can be > checked when building Sage's Python SPKG with SAGE_CHECK=yes. > > Otherwise, I don't think the Sage test suite has any business testing > this. > > I think the idea of patching Python's test suite is a nice one, but in my experience, the test suite fails already, so we should first get the test suite to pass, and then remove it from the default list of excluded packages for testing (see build/bin/sage-spkg). Then we could patch the test suite. -- John -- You received this message because you are subscribed to the Google Groups "sage-devel" group. To unsubscribe from this group and stop receiving emails from it, send an email to sage-devel+unsubscr...@googlegroups.com. To post to this group, send email to sage-devel@googlegroups.com. Visit this group at https://groups.google.com/group/sage-devel. For more options, visit https://groups.google.com/d/optout.
Re: [sage-devel] What to do with the sys.path security patch?
> On 1/03/2019, at 08:40, Timo Kaufmann wrote: > > I suggest a middle ground: I don't believe this behavior should be > tested in Sage's test suite, because this is a question about the > Python interpreter's behavior, not Sage. > [...] > Otherwise, I don't think the Sage test suite has any business testing this. > > +1, I agree with everything Erik (or do you prefer Madison?) said. > > This can be a legitimate privilege escalation issue, but sage shouldn't test > for upstream behaviour that it doesn't even rely on. I'm neutral on adding > the patches, but I would prefer the tests to be removed. > +1 I have been removing the two corresponding doctests (one in dockets/control.py and a second one in tests/cmdline.py) since at sage 7.3 in sage-on-gentoo. -- You received this message because you are subscribed to the Google Groups "sage-devel" group. To unsubscribe from this group and stop receiving emails from it, send an email to sage-devel+unsubscr...@googlegroups.com. To post to this group, send email to sage-devel@googlegroups.com. Visit this group at https://groups.google.com/group/sage-devel. For more options, visit https://groups.google.com/d/optout.
Re: [sage-devel] What to do with the sys.path security patch?
There is some previous discussion here: https://trac.sagemath.org/ticket/26457 -- You received this message because you are subscribed to the Google Groups "sage-devel" group. To unsubscribe from this group and stop receiving emails from it, send an email to sage-devel+unsubscr...@googlegroups.com. To post to this group, send email to sage-devel@googlegroups.com. Visit this group at https://groups.google.com/group/sage-devel. For more options, visit https://groups.google.com/d/optout.
Re: [sage-devel] What to do with the sys.path security patch?
> > I suggest a middle ground: I don't believe this behavior should be > tested in Sage's test suite, because this is a question about the > Python interpreter's behavior, not Sage. > [...] > Otherwise, I don't think the Sage test suite has any business testing > this. > +1, I agree with everything Erik (or do you prefer Madison?) said. This can be a legitimate privilege escalation issue, but sage shouldn't test for upstream behaviour that it doesn't even rely on. I'm neutral on adding the patches, but I would prefer the tests to be removed. -- You received this message because you are subscribed to the Google Groups "sage-devel" group. To unsubscribe from this group and stop receiving emails from it, send an email to sage-devel+unsubscr...@googlegroups.com. To post to this group, send email to sage-devel@googlegroups.com. Visit this group at https://groups.google.com/group/sage-devel. For more options, visit https://groups.google.com/d/optout.
Re: [sage-devel] What to do with the sys.path security patch?
On Thu, Feb 28, 2019 at 4:53 PM Jeroen Demeyer wrote: > > Since many years, we have patched the Python in Sage with a patch to > mitigate some security issues related to sys.path. > > The security issue is the following: if you run a Python script from a > world-writable directory, say a script like /tmp/foo.py this is very > insecure since Python will add /tmp as first entry in sys.path. So if > the script starts with from __future__ import absolute_import (say) and > some evil person creates /tmp/__future__.py then you'll run that code. > > We added a patch for this in Sage in > https://trac.sagemath.org/ticket/13579 and also reported this upstream > to CPython at https://bugs.python.org/issue16202 > > As expected, upstream was not impressed by our patch because it would > break backwards compatibility. But we never really tried to discuss it > seriously with upstream. If anybody wants to go for that, please do. I > won't do this, I've already picked different battles with CPython. > > As a side remark, let me mention that Perl also had this issue but they > fixed it, the current directory is no longer in the include path: > https://metacpan.org/pod/release/XSAWYERX/perl-5.26.0/pod/perldelta.pod#Removal-of-the-current-directory-%28%22.%22%29-from-@INC > > I noticed at Trac #27001 that we don't have this patch in Python 3. The > question is: should we? It makes little sense to have it for Python 2 > and not for Python 3. So either we drop it from Python 2 or we port it > to Python 3. The ideal situation would be that it gets accepted > upstream, but for the sake of argument, let's assume that it doesn't get > accepted. What should we do then? I suggest a middle ground: I don't believe this behavior should be tested in Sage's test suite, because this is a question about the Python interpreter's behavior, not Sage. The possibility of running Sage on a Python interpreter provided by the system, which does not have this patch, means the behavior here cannot be controlled for. So I think if you want to include this patch in Sage, the patch should include patches to Python's own test suite (as would be needed in any case to have any chance of getting it upstream) so that it can be checked when building Sage's Python SPKG with SAGE_CHECK=yes. Otherwise, I don't think the Sage test suite has any business testing this. I don't care about porting the patch to Python 3 either. Though if someone did, and raised the issue on python-dev, I think there's not a terrible chance of getting this accepted. Yes, it would break some backwards compatibility, but for a use case that isn't very strong in the first place. So I could see something like that getting accepted in a future Python 3.x release (and likely never for Python 2.7, though you could make an argument for it with various distros if it's something you're passionate about :) By "you" here I mean anyone reading btw, not just Jeroen. -- You received this message because you are subscribed to the Google Groups "sage-devel" group. To unsubscribe from this group and stop receiving emails from it, send an email to sage-devel+unsubscr...@googlegroups.com. To post to this group, send email to sage-devel@googlegroups.com. Visit this group at https://groups.google.com/group/sage-devel. For more options, visit https://groups.google.com/d/optout.
[sage-devel] What to do with the sys.path security patch?
Since many years, we have patched the Python in Sage with a patch to mitigate some security issues related to sys.path. The security issue is the following: if you run a Python script from a world-writable directory, say a script like /tmp/foo.py this is very insecure since Python will add /tmp as first entry in sys.path. So if the script starts with from __future__ import absolute_import (say) and some evil person creates /tmp/__future__.py then you'll run that code. We added a patch for this in Sage in https://trac.sagemath.org/ticket/13579 and also reported this upstream to CPython at https://bugs.python.org/issue16202 As expected, upstream was not impressed by our patch because it would break backwards compatibility. But we never really tried to discuss it seriously with upstream. If anybody wants to go for that, please do. I won't do this, I've already picked different battles with CPython. As a side remark, let me mention that Perl also had this issue but they fixed it, the current directory is no longer in the include path: https://metacpan.org/pod/release/XSAWYERX/perl-5.26.0/pod/perldelta.pod#Removal-of-the-current-directory-%28%22.%22%29-from-@INC I noticed at Trac #27001 that we don't have this patch in Python 3. The question is: should we? It makes little sense to have it for Python 2 and not for Python 3. So either we drop it from Python 2 or we port it to Python 3. The ideal situation would be that it gets accepted upstream, but for the sake of argument, let's assume that it doesn't get accepted. What should we do then? Jeroen. -- You received this message because you are subscribed to the Google Groups "sage-devel" group. To unsubscribe from this group and stop receiving emails from it, send an email to sage-devel+unsubscr...@googlegroups.com. To post to this group, send email to sage-devel@googlegroups.com. Visit this group at https://groups.google.com/group/sage-devel. For more options, visit https://groups.google.com/d/optout.
