Re: [PATCH stable-2.15 13/37] Fix pylint >1.4 pycurl no-member warnings

['Iustin Pop' via ganeti-devel]

On 5 December 2016 at 14:56, 'Brian Foley' via ganeti-devel <
ganeti-devel@googlegroups.com> wrote:

> On Mon, Dec 05, 2016 at 01:43:57PM +, Federico Pareschi wrote:
> >  to avoid arbitrary code injection.
> >
> >Is this safe? Should we be looking more into this or is it something
> >that does not affect us?
>
> The attack vector is someone writing their own pycurl.so with malicious
> code
> in it, puting that on the python search path, and using that to perform
> arbitrary actions as the user running pylint when pylint loads pycurl.
>
> I think this is a legitimate concern for developers running pylint as
> themselves and potentially accepting arbitary patches from the internet
> without
> checking what's in them, but it shouldn't be much of a problem for a
> sandboxed
> buildbot.
>

Even for developers, random patches can't easily ship a .so, so while this
is a valid concern for some cases, I think it's less likely for accepting
source diffs (that show no binary files) and linting them.

I wonder if it's possible to write a git config that rejects binary deltas…

The only alternatives I can think of are to explicitly disable the warning
> for
> any ganeti modules that use pycurl directly, or to explicitly add disables
> to
> each use of pycurl.foo but I don't much like those.
>

I tend to agree, this seems a worthwhile tradeoff.

iustin

Re: [PATCH stable-2.15 13/37] Fix pylint >1.4 pycurl no-member warnings

['Brian Foley' via ganeti-devel]

On Mon, Dec 05, 2016 at 01:43:57PM +, Federico Pareschi wrote:
>  to avoid arbitrary code injection.
> 
>Is this safe? Should we be looking more into this or is it something
>that does not affect us?

The attack vector is someone writing their own pycurl.so with malicious code
in it, puting that on the python search path, and using that to perform
arbitrary actions as the user running pylint when pylint loads pycurl.

I think this is a legitimate concern for developers running pylint as
themselves and potentially accepting arbitary patches from the internet without
checking what's in them, but it shouldn't be much of a problem for a sandboxed
buildbot.

The only alternatives I can think of are to explicitly disable the warning for
any ganeti modules that use pycurl directly, or to explicitly add disables to
each use of pycurl.foo but I don't much like those.

Cheers,
Brian.

Re: [PATCH stable-2.15 13/37] Fix pylint >1.4 pycurl no-member warnings

['Federico Pareschi' via ganeti-devel]

>
> to avoid arbitrary code injection.
>

Is this safe? Should we be looking more into this or is it something that
does not affect us?

[PATCH stable-2.15 13/37] Fix pylint >1.4 pycurl no-member warnings

['Brian Foley' via ganeti-devel]

Add pycurl to the whitelist of C extensions that can be loaded for the
purposes of attribute checking. This works around a security feature
introduced in pylint 1.4 to avoid arbitrary code injection.

Signed-off-by: Brian Foley 
---
 Makefile.am  | 4 
 configure.ac | 4 
 2 files changed, 8 insertions(+)

diff --git a/Makefile.am b/Makefile.am
index 0b30033..8b859c1 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -2692,6 +2692,10 @@ LINT_OPTS =
 # The combined set of pylint options
 LINT_OPTS_ALL = $(LINT_OPTS) \
   $(addprefix --disable=,$(LINT_DISABLE))
+if !PYLINT_LT_14
+# Whitelist loading pycurl C extension for attribute checking
+LINT_OPTS_ALL += --extension-pkg-whitelist=pycurl
+endif
 
 LINT_TARGETS = pylint pylint-qa pylint-test
 if HAS_PEP8
diff --git a/configure.ac b/configure.ac
index 9b5d06f..f57b98c 100644
--- a/configure.ac
+++ b/configure.ac
@@ -576,6 +576,10 @@ then
   AC_MSG_WARN([pylint not found, checking code will not be possible])
 fi
 
+# Note: Character classes ([...]) need to be double quoted due to autoconf
+# using m4
+AM_CONDITIONAL([PYLINT_LT_14], [$PYLINT --version | grep -q '^pylint 
\(0\.[[0-9]]\|1\.[[0-3]]\.'])
+
 # Check for pep8
 AC_ARG_VAR(PEP8, [pep8 path])
 AC_PATH_PROG(PEP8, [pep8], [])
-- 
2.8.0.rc3.226.g39d4020