From 53f7f0284c084ac2e4542fd1f71d0406075adb5d Mon Sep 17 00:00:00 2001
From: Nir Soffer <nsoffer@redhat.com>
Date: Jun 25 2019 15:13:19 +0000
Subject: python: Fail with ValueError if disk path is too long


Previously if a path was too long, we truncated the path silently. This
probably fail when sanlock try to access non-existing path with "No such
path or directory" error. If you are unlucky, this could try to access
the wrong path, failing with bogus error, returning wrong data, or
worse, writing a resource into the wrong location, destroying existing
data.

Now we fail immediately with:

    ValueError: Path is too long: '/too/long/path/...'

Signed-off-by: Nir Soffer <nsoffer@redhat.com>

---

diff --git a/python/sanlock.c b/python/sanlock.c
index ca3713d..7c50aba 100644
--- a/python/sanlock.c
+++ b/python/sanlock.c
@@ -146,6 +146,17 @@ pystring_as_cstring(PyObject *obj)
 }
 
 static int
+validate_path(PyObject *path)
+{
+    if (PyBytes_Size(path) > SANLK_PATH_LEN - 1) {
+        set_error(PyExc_ValueError, "Path is too long: %s", path);
+        return 0;
+    }
+
+    return 1;
+}
+
+static int
 parse_single_disk(PyObject* disk, struct sanlk_disk* res_disk)
 {
     int rv = 0;
@@ -163,6 +174,9 @@ parse_single_disk(PyObject* disk, struct sanlk_disk* res_disk)
         goto finally;
     }
 
+    if (!validate_path(path))
+        goto finally;
+
     strncpy(res_disk->path, PyBytes_AsString(path), SANLK_PATH_LEN - 1);
     res_disk->offset = offset;
     rv = 1;
@@ -656,6 +670,9 @@ py_read_resource(PyObject *self __unused, PyObject *args, PyObject *keywds)
         goto finally;
     }
 
+    if (!validate_path(path))
+        goto finally;
+
     /* prepare the resource disk path */
     strncpy(res->disks[0].path, PyBytes_AsString(path), SANLK_PATH_LEN - 1);
 
diff --git a/tests/python_test.py b/tests/python_test.py
index 4ea1589..719bcb6 100644
--- a/tests/python_test.py
+++ b/tests/python_test.py
@@ -601,7 +601,6 @@ def test_write_resource_parse_args(
         sanlock.write_resource(b"ls_name", name, disks)
 
 
-@pytest.mark.xfail(reason="path truncated silently")
 def test_write_resource_path_length(no_sanlock_daemon):
     path = "x" * constants.SANLK_PATH_LEN
     with pytest.raises(ValueError):
@@ -625,7 +624,6 @@ def test_release_resource_parse_args(
         sanlock.release(b"ls_name", name, disks)
 
 
-@pytest.mark.xfail(reason="path truncated silently")
 def test_release_resource_path_length(no_sanlock_daemon):
     path = "x" * constants.SANLK_PATH_LEN
     with pytest.raises(ValueError):
@@ -649,7 +647,6 @@ def test_read_resource_owners_parse_args(
         sanlock.read_resource_owners(b"ls_name", name, disks)
 
 
-@pytest.mark.xfail(reason="path truncated silently")
 def test_read_resource_owners_path_length(no_sanlock_daemon):
     path = "x" * constants.SANLK_PATH_LEN
     with pytest.raises(ValueError):
@@ -712,7 +709,6 @@ def test_init_resource_parse_args(no_sanlock_daemon, name, filename, encoding):
         sanlock.init_resource(name, b"res_name", disks)
 
 
-@pytest.mark.xfail(reason="path truncated silently")
 def test_init_resource_path_length(no_sanlock_daemon):
     path = "x" * constants.SANLK_PATH_LEN
     with pytest.raises(ValueError):
@@ -745,7 +741,6 @@ def test_read_resource_parse_args(no_sanlock_daemon, filename, encoding):
         sanlock.read_resource(path)
 
 
-@pytest.mark.xfail(reason="path truncated silently")
 def test_read_resource_path_length(no_sanlock_daemon):
     path = "x" * constants.SANLK_PATH_LEN
     with pytest.raises(ValueError):
@@ -769,7 +764,6 @@ def test_request_parse_args(no_sanlock_daemon, name, filename, encoding):
         sanlock.request(name, b"res_name", disks)
 
 
-@pytest.mark.xfail(reason="path truncated silently")
 def test_request_path_length(no_sanlock_daemon):
     path = "x" * constants.SANLK_PATH_LEN
     with pytest.raises(ValueError):
@@ -793,7 +787,6 @@ def test_acquire_parse_args(no_sanlock_daemon, name, filename, encoding):
         sanlock.acquire(name, b"res_name", disks, pid=os.getpid())
 
 
-@pytest.mark.xfail(reason="path truncated silently")
 def test_acquire_path_length(no_sanlock_daemon):
     path = "x" * constants.SANLK_PATH_LEN
     with pytest.raises(ValueError):