[PATCH] check types rather than segfault in pygpo.c

Douglas Bagnall douglas.bagnall at catalyst.net.nz
Fri Apr 13 05:36:05 UTC 2018


On 13/04/18 14:45, Andrew Bartlett via samba-technical wrote:
> On Fri, 2018-04-13 at 14:39 +1200, Douglas Bagnall via samba-technical
> wrote:
>> I was trying to find good examples for another thread, and only found bad.
>>
>> Douglas
> 
> Thanks Douglas!
> 
> Reviewed-by: Andrew Bartlett <abartlet at samba.org>
>
> Please push!

Actually it is better like this, with the test squashed into the fix (or
I guess omitted altogether), because the knownfail entry doesn't cope
with the segfault, so we'd have a failure between those commits.

Douglas
-------------- next part --------------
From 0ffb08b3054f3ab4e78d1f2ae382e7164057c1a9 Mon Sep 17 00:00:00 2001
From: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date: Fri, 13 Apr 2018 12:29:05 +1200
Subject: [PATCH] python.gpo.ADS_STRUCT: check type of loadparm argument

And add a test showning the segfault.

Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
---
 libgpo/pygpo.c            | 9 ++++++++-
 python/samba/tests/gpo.py | 7 +++++++
 2 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/libgpo/pygpo.c b/libgpo/pygpo.c
index db336021125..60220a6bc2a 100644
--- a/libgpo/pygpo.c
+++ b/libgpo/pygpo.c
@@ -183,7 +183,14 @@ static int py_ads_init(ADS *self, PyObject *args, PyObject *kwds)
 	}
 
 	if (lp_obj) {
-		bool ok;
+		bool ok = py_check_dcerpc_type(lp_obj, "samba.param",
+					       "LoadParm");
+		if (!ok) {
+			PyErr_Format(PyExc_TypeError,
+				     "Expected samba.param.LoadParm "
+				     "for lp argument");
+			return -1;
+		}
 		lp_ctx = pytalloc_get_type(lp_obj, struct loadparm_context);
 		if (lp_ctx == NULL) {
 			return -1;
diff --git a/python/samba/tests/gpo.py b/python/samba/tests/gpo.py
index bdcbcc6c754..796a5cb06cb 100644
--- a/python/samba/tests/gpo.py
+++ b/python/samba/tests/gpo.py
@@ -50,6 +50,13 @@ class GPOTests(tests.TestCase):
             assert gpos[i].ds_path == ds_paths[i], \
               'ds_path did not match expected %s' % gpos[i].ds_path
 
+
+    def test_gpo_ads_does_not_segfault(self):
+        try:
+            ads = gpo.ADS_STRUCT(self.server, 42, self.creds)
+        except:
+            pass
+
     def test_gpt_version(self):
         global gpt_data
         local_path = self.lp.get("path", "sysvol")
-- 
2.14.1



More information about the samba-technical mailing list