Patch 0001: Handle off-by-one error in path_concat() Fixes https://fedorahosted.org/sssd/ticket/1230
Patch 0002: While fixing the above error, I noticed that path_concat() would also get it wrong if we passed "/" for head.
From 0231e3f60c465517e6f6296de7edc1cf10fe6fc7 Mon Sep 17 00:00:00 2001 From: Stephen Gallagher <sgall...@redhat.com> Date: Tue, 6 Mar 2012 11:05:53 -0500 Subject: [PATCH 1/2] path_utils: handle off-by-one error in path_concat() https://bugzilla.redhat.com/show_bug.cgi?id=799347 --- path_utils/path_utils.c | 2 +- path_utils/path_utils_ut.c | 18 ++++++++++++++---- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/path_utils/path_utils.c b/path_utils/path_utils.c index 97c845c70f41e071c0d9524172ad3b0b33a84c3b..360d49943436681970691cc829c8039f3a0e8a5f 100644 --- a/path_utils/path_utils.c +++ b/path_utils/path_utils.c @@ -210,7 +210,7 @@ int path_concat(char *path, size_t path_size, const char *head, const char *tail for (p = tail; *p && *p == '/'; p++); /* skip any leading slashes in tail */ if (dst > path) if (dst < dst_end) *dst++ = '/'; /* insert single slash between head & tail */ - for (src = p; *src && dst <= dst_end;) *dst++ = *src++; /* copy tail */ + for (src = p; *src && dst < dst_end;) *dst++ = *src++; /* copy tail */ if (*src) return ENOBUFS; /* failed to copy everything */ } *dst = 0; diff --git a/path_utils/path_utils_ut.c b/path_utils/path_utils_ut.c index 044e9ea2d12e396f9f68a23bde8b11f6ac3fcc09..fbefcab1eb66b5c36a3d7a74a099f569ea764b3b 100644 --- a/path_utils/path_utils_ut.c +++ b/path_utils/path_utils_ut.c @@ -241,16 +241,26 @@ END_TEST START_TEST(test_path_concat_neg) { char small[3]; - char small2[4]; - char p2[8]; + char small2[5]; + char small3[7]; + char p2[9]; /* these two test different conditions */ + + /* Test if head is longer than the buffer */ fail_unless(path_concat(small, 3, "/foo", "bar") == ENOBUFS); - fail_unless(path_concat(small2, 4, "/foo", "bar") == ENOBUFS); + + /* Test if head is the same length as the buffer */ + fail_unless(path_concat(small2, 5, "/foo", "bar") == ENOBUFS); + + /* Test if head+tail is the longer than the buffer */ + fail_unless(path_concat(small3, 7, "/foo", "bar") == ENOBUFS); /* off-by-one */ + p2[8] = 1; /* Check whether we've written too far */ fail_unless(path_concat(p2, 8, "/foo", "bar") == ENOBUFS); - fail_unless_str_equal(p2, "/foo/bar"); + fail_unless(p2[8] == 1); /* This should be untouched */ + fail_unless_str_equal(p2, "/foo/ba"); } END_TEST -- 1.7.7.6
From 88d8a574525dbe71671127b8dc034f35e4bb1638 Mon Sep 17 00:00:00 2001 From: Stephen Gallagher <sgall...@redhat.com> Date: Tue, 6 Mar 2012 11:34:41 -0500 Subject: [PATCH 2/2] path_utils: Handle "/" in path_compat --- path_utils/path_utils.c | 12 ++++++++++-- path_utils/path_utils_ut.c | 9 +++++++++ 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/path_utils/path_utils.c b/path_utils/path_utils.c index 360d49943436681970691cc829c8039f3a0e8a5f..25ca4267fc0d7d9f3483646c2aef22fcdef1eb24 100644 --- a/path_utils/path_utils.c +++ b/path_utils/path_utils.c @@ -202,14 +202,22 @@ int path_concat(char *path, size_t path_size, const char *head, const char *tail if (head && *head) { for (p = head; *p; p++); /* walk to end of head */ - for (p--; p >= head && *p == '/'; p--); /* skip any trailing slashes in head */ + for (p--; p > head && *p == '/'; p--); /* skip any trailing slashes in head */ if ((p - head) > path_size-1) return ENOBUFS; for (src = head; src <= p && dst < dst_end;) *dst++ = *src++; /* copy head */ } if (tail && *tail) { for (p = tail; *p && *p == '/'; p++); /* skip any leading slashes in tail */ if (dst > path) - if (dst < dst_end) *dst++ = '/'; /* insert single slash between head & tail */ + /* insert single slash between head & tail + * Making sure not to add an extra if the + * preceding character is also a slash + * (such as the case where head was the + * special-case "/". + */ + if (dst < dst_end && *(dst-1) != '/') { + *dst++ = '/'; + } for (src = p; *src && dst < dst_end;) *dst++ = *src++; /* copy tail */ if (*src) return ENOBUFS; /* failed to copy everything */ } diff --git a/path_utils/path_utils_ut.c b/path_utils/path_utils_ut.c index fbefcab1eb66b5c36a3d7a74a099f569ea764b3b..34462cfdd4a2238f878a170fba2481b31f96e33e 100644 --- a/path_utils/path_utils_ut.c +++ b/path_utils/path_utils_ut.c @@ -229,6 +229,15 @@ START_TEST(test_path_concat) fail_unless(path_concat(p, PATH_MAX, "", "foo") == SUCCESS); fail_unless_str_equal(p, "foo"); + fail_unless(path_concat(p, PATH_MAX, "/", "foo") == SUCCESS); + fail_unless_str_equal(p, "/foo"); + + fail_unless(path_concat(p, PATH_MAX, "/foo", "/") == SUCCESS); + fail_unless_str_equal(p, "/foo/"); + + fail_unless(path_concat(p, PATH_MAX, "/foo", "bar/") == SUCCESS); + fail_unless_str_equal(p, "/foo/bar/"); + fail_unless(path_concat(p, PATH_MAX, NULL, "foo") == SUCCESS); fail_unless_str_equal(p, "foo"); -- 1.7.7.6
signature.asc
Description: This is a digitally signed message part
_______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/sssd-devel