Matteo Cafasso
2017-May-11 18:21 UTC
[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@gmail.com>
---
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
Pino Toscano
2017-May-15 12:36 UTC
Re: [Libguestfs] [PATCH v2] RHBZ#1406906: check return value of Python object functions
On Thursday, 11 May 2017 20:21:39 CEST Matteo Cafasso wrote:> 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@gmail.com> > ---LGTM -- the only thing to change is the first line of the commit message, which should be like "python: ... (RHBZ#...)". Not going to ask you to respin a v3 just because of that, though. Amended, and pushed. I will close the bug soon too. Thanks, -- Pino Toscano
Apparently Analagous Threads
- [PATCH] RHBZ#1406906: check return value of Python object functions
- [PATCH] python: add simple wrappers for PyObject<->string functions
- [PATCH v2] python: add simple wrappers for PyObject<->string functions
- [Bug #1406906] [PATCH] python: fix segmentation fault when setting non UTF-8 strings
- [PATCH] python: check return value of Python APIs