[Libguestfs] [PATCH v2] RHBZ#1406906: check return value of Python object functions

Verify the returned values of Python Object constructor functions are not NULL before adding them to a collection. This is particularly relevant when constructing Unicode strings in Python 3 as they will return NULL if non UTF-8 characters are present. Signed-off-by: Matteo Cafasso <noxdafox(a)gmail.com&gt; --- generator/python.ml | 102 ++++++++++++++++++++++++++--------------- python/handle.c | 36 ++++++++++++--- python/t/test830RHBZ1406906.py | 57 +++++++++++++++++++++++ 3 files changed, 152 insertions(+), 43 deletions(-) create mode 100644 python/t/test830RHBZ1406906.py diff --git a/generator/python.ml b/generator/python.ml index cf0829489..4cae24757 100644 --- a/generator/python.ml +++ b/generator/python.ml @@ -155,12 +155,20 @@ and generate_python_structs () = pr "PyObject *\n"; pr "guestfs_int_py_put_%s_list (struct guestfs_%s_list *%ss)\n" typ typ typ; pr "{\n"; - pr " PyObject *list;\n"; + pr " PyObject *list, *element;\n"; pr " size_t i;\n"; pr "\n"; pr " list = PyList_New (%ss->len);\n" typ; - pr " for (i = 0; i < %ss->len; ++i)\n" typ; - pr " PyList_SetItem (list, i, guestfs_int_py_put_%s (&%ss->val[i]));\n" typ typ; + pr " if (list == NULL)\n"; + pr " return NULL;\n"; + pr " for (i = 0; i < %ss->len; ++i) {\n" typ; + pr " element = guestfs_int_py_put_%s (&%ss->val[i]);\n" typ typ; + pr " if (element == NULL) {\n"; + pr " Py_CLEAR (list);\n"; + pr " return NULL;\n"; + pr " }\n"; + pr " PyList_SetItem (list, i, element);\n"; + pr " }\n"; pr " return list;\n"; pr "};\n"; pr "#endif\n"; @@ -174,54 +182,72 @@ and generate_python_structs () = pr "PyObject *\n"; pr "guestfs_int_py_put_%s (struct guestfs_%s *%s)\n" typ typ typ; pr "{\n"; - pr " PyObject *dict;\n"; + pr " PyObject *dict, *value;\n"; pr "\n"; pr " dict = PyDict_New ();\n"; + pr " if (dict == NULL)\n"; + pr " return NULL;\n"; List.iter ( function | name, FString -> - pr " PyDict_SetItemString (dict, \"%s\",\n" name; - pr " guestfs_int_py_fromstring (%s->%s));\n" - typ name + pr " value = guestfs_int_py_fromstring (%s->%s);" typ name; + pr " if (value == NULL)\n"; + pr " goto err;\n"; + pr " PyDict_SetItemString (dict, \"%s\", value);\n" name; | name, FBuffer -> - pr " PyDict_SetItemString (dict, \"%s\",\n" name; - pr " guestfs_int_py_fromstringsize (%s->%s, %s->%s_len));\n" - typ name typ name + pr " value = guestfs_int_py_fromstringsize (%s->%s, %s->%s_len);\n" + typ name typ name; + pr " if (value == NULL)\n"; + pr " goto err;\n"; + pr " PyDict_SetItemString (dict, \"%s\", value);\n" name; | name, FUUID -> - pr " PyDict_SetItemString (dict, \"%s\",\n" name; - pr " guestfs_int_py_fromstringsize (%s->%s, 32));\n" - typ name + pr " value = guestfs_int_py_fromstringsize (%s->%s, 32);\n" + typ name; + pr " if (value == NULL)\n"; + pr " goto err;\n"; + pr " PyDict_SetItemString (dict, \"%s\", value);\n" name; | name, (FBytes|FUInt64) -> - pr " PyDict_SetItemString (dict, \"%s\",\n" name; - pr " PyLong_FromUnsignedLongLong (%s->%s));\n" - typ name + pr " value = PyLong_FromUnsignedLongLong (%s->%s);\n" typ name; + pr " if (value == NULL)\n"; + pr " goto err;\n"; + pr " PyDict_SetItemString (dict, \"%s\", value);\n" name; | name, FInt64 -> - pr " PyDict_SetItemString (dict, \"%s\",\n" name; - pr " PyLong_FromLongLong (%s->%s));\n" - typ name + pr " value = PyLong_FromLongLong (%s->%s);\n" typ name; + pr " if (value == NULL)\n"; + pr " goto err;\n"; + pr " PyDict_SetItemString (dict, \"%s\", value);\n" name; | name, FUInt32 -> - pr " PyDict_SetItemString (dict, \"%s\",\n" name; - pr " PyLong_FromUnsignedLong (%s->%s));\n" - typ name + pr " value = PyLong_FromUnsignedLong (%s->%s);\n" typ name; + pr " if (value == NULL)\n"; + pr " goto err;\n"; + pr " PyDict_SetItemString (dict, \"%s\", value);\n" name; | name, FInt32 -> - pr " PyDict_SetItemString (dict, \"%s\",\n" name; - pr " PyLong_FromLong (%s->%s));\n" - typ name + pr " value = PyLong_FromLong (%s->%s);\n" typ name; + pr " if (value == NULL)\n"; + pr " goto err;\n"; + pr " PyDict_SetItemString (dict, \"%s\", value);\n" name; | name, FOptPercent -> - pr " if (%s->%s >= 0)\n" typ name; - pr " PyDict_SetItemString (dict, \"%s\",\n" name; - pr " PyFloat_FromDouble ((double) %s->%s));\n" - typ name; + pr " if (%s->%s >= 0) {\n" typ name; + pr " value = PyFloat_FromDouble ((double) %s->%s);\n" typ name; + pr " if (value == NULL)\n"; + pr " goto err;\n"; + pr " PyDict_SetItemString (dict, \"%s\", value);\n" name; + pr " }\n"; pr " else {\n"; pr " Py_INCREF (Py_None);\n"; pr " PyDict_SetItemString (dict, \"%s\", Py_None);\n" name; pr " }\n" | name, FChar -> - pr " PyDict_SetItemString (dict, \"%s\",\n" name; - pr " guestfs_int_py_fromstringsize (&%s->%s, 1));\n" - typ name + pr " value = guestfs_int_py_fromstringsize (&%s->%s, 1);\n" + typ name; + pr " if (value == NULL)\n"; + pr " goto err;\n"; + pr " PyDict_SetItemString (dict, \"%s\", value);\n" name; ) cols; pr " return dict;\n"; + pr " err:\n"; + pr " Py_CLEAR (dict);\n"; + pr " return NULL;\n"; pr "};\n"; pr "#endif\n"; pr "\n"; @@ -472,16 +498,20 @@ and generate_python_actions actions () = pr " if (py_r == NULL) goto out;\n"; | RStringList _ -> pr " py_r = guestfs_int_py_put_string_list (r);\n"; - pr " guestfs_int_free_string_list (r);\n" + pr " guestfs_int_free_string_list (r);\n"; + pr " if (py_r == NULL) goto out;\n"; | RStruct (_, typ) -> pr " py_r = guestfs_int_py_put_%s (r);\n" typ; - pr " guestfs_free_%s (r);\n" typ + pr " guestfs_free_%s (r);\n" typ; + pr " if (py_r == NULL) goto out;\n"; | RStructList (_, typ) -> pr " py_r = guestfs_int_py_put_%s_list (r);\n" typ; - pr " guestfs_free_%s_list (r);\n" typ + pr " guestfs_free_%s_list (r);\n" typ; + pr " if (py_r == NULL) goto out;\n"; | RHashtable _ -> pr " py_r = guestfs_int_py_put_table (r);\n"; - pr " guestfs_int_free_string_list (r);\n" + pr " guestfs_int_free_string_list (r);\n"; + pr " if (py_r == NULL) goto out;\n"; | RBufferOut _ -> pr " py_r = guestfs_int_py_fromstringsize (r, size);\n"; pr " free (r);\n"; diff --git a/python/handle.c b/python/handle.c index 88024e184..d93f2f021 100644 --- a/python/handle.c +++ b/python/handle.c @@ -324,15 +324,23 @@ guestfs_int_py_get_string_list (PyObject *obj) PyObject * guestfs_int_py_put_string_list (char * const * const argv) { - PyObject *list; + PyObject *list, *item; size_t argc, i; for (argc = 0; argv[argc] != NULL; ++argc) ; list = PyList_New (argc); + if (list == NULL) + return NULL; for (i = 0; i < argc; ++i) { - PyList_SetItem (list, i, guestfs_int_py_fromstring (argv[i])); + item = guestfs_int_py_fromstring (argv[i]); + if (item == NULL) { + Py_CLEAR (list); + return NULL; + } + + PyList_SetItem (list, i, item); } return list; @@ -341,21 +349,35 @@ guestfs_int_py_put_string_list (char * const * const argv) PyObject * guestfs_int_py_put_table (char * const * const argv) { - PyObject *list, *item; + PyObject *list, *tuple, *item; size_t argc, i; for (argc = 0; argv[argc] != NULL; ++argc) ; list = PyList_New (argc >> 1); + if (list == NULL) + return NULL; for (i = 0; i < argc; i += 2) { - item = PyTuple_New (2); - PyTuple_SetItem (item, 0, guestfs_int_py_fromstring (argv[i])); - PyTuple_SetItem (item, 1, guestfs_int_py_fromstring (argv[i+1])); - PyList_SetItem (list, i >> 1, item); + tuple = PyTuple_New (2); + if (tuple == NULL) + goto err; + item = guestfs_int_py_fromstring (argv[i]); + if (item == NULL) + goto err; + PyTuple_SetItem (tuple, 0, item); + item = guestfs_int_py_fromstring (argv[i+1]); + if (item == NULL) + goto err; + PyTuple_SetItem (tuple, 1, item); + PyList_SetItem (list, i >> 1, tuple); } return list; + err: + Py_CLEAR (list); + Py_CLEAR (tuple); + return NULL; } PyObject * diff --git a/python/t/test830RHBZ1406906.py b/python/t/test830RHBZ1406906.py new file mode 100644 index 000000000..17b875226 --- /dev/null +++ b/python/t/test830RHBZ1406906.py @@ -0,0 +1,57 @@ +# libguestfs Python bindings +# Copyright (C) 2017 Red Hat Inc. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + +import os +import sys +import shutil +import tempfile +import unittest + +import guestfs + + +class Test830RHBZ1406906(unittest.TestCase): + def setUp(self): + self.tempdir = tempfile.mkdtemp() + + def tearDown(self): + shutil.rmtree(self.tempdir) + + def test_rhbz1406906(self): + g = guestfs.GuestFS(python_return_dict=True) + + g.add_drive_scratch(512 * 1024 * 1024) + g.launch() + + g.part_disk("/dev/sda", "mbr") + g.mkfs("ext4", "/dev/sda1") + g.mount("/dev/sda1", "/") + + self.assertEqual(g.find("/"), ['lost+found']) + + # touch file with illegal unicode character + non_utf8_fname = "\udcd4" + open(os.path.join(self.tempdir, non_utf8_fname), "w").close() + + g.copy_in(self.tempdir, "/") + + if sys.version_info >= (3, 0): + with self.assertRaises(UnicodeDecodeError): + g.find("/") # segfault here on Python 3 + elif sys.version_info >= (2, 0): + self.assertTrue( + any(path for path in g.find("/") if non_utf8_fname in path)) -- 2.11.0