[pacman-dev] [PATCH 5/7] fix off-by-one error in _alpm_filelist_resolve
Signed-off-by: Andrew Gregory <andrew.gregory.8@gmail.com> --- lib/libalpm/filelist.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/lib/libalpm/filelist.c b/lib/libalpm/filelist.c index f29bb7c..c2ca1ca 100644 --- a/lib/libalpm/filelist.c +++ b/lib/libalpm/filelist.c @@ -198,12 +198,15 @@ int _alpm_filelist_resolve(alpm_handle_t *handle, alpm_filelist_t *files) if(realpath(handle->root, path) == NULL){ return -1; } - root_len = strlen(path) + 1; - if(root_len >= PATH_MAX) { + root_len = strlen(path); + if(root_len + 1 >= PATH_MAX) { return -1; } - path[root_len - 1] = '/'; - path[root_len] = '\0'; + if(path[root_len - 1] != '/') { + path[root_len] = '/'; + root_len++; + path[root_len] = '\0'; + } ret = _alpm_filelist_resolve_link(files, &i, path, root_len, 0); -- 1.8.1.3
On 14/02/13 09:54, Andrew Gregory wrote:
Signed-off-by: Andrew Gregory <andrew.gregory.8@gmail.com> ---
OK... I'm tired, so feel free to point out where I am wring here... AFAIK, realpath will not return a trailing "/". So what is being fixed here? If I take out the unneeded if statement, if am failing to see the off-by-one error...
lib/libalpm/filelist.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/lib/libalpm/filelist.c b/lib/libalpm/filelist.c index f29bb7c..c2ca1ca 100644 --- a/lib/libalpm/filelist.c +++ b/lib/libalpm/filelist.c @@ -198,12 +198,15 @@ int _alpm_filelist_resolve(alpm_handle_t *handle, alpm_filelist_t *files) if(realpath(handle->root, path) == NULL){ return -1; } - root_len = strlen(path) + 1; - if(root_len >= PATH_MAX) { + root_len = strlen(path); + if(root_len + 1 >= PATH_MAX) { return -1; } - path[root_len - 1] = '/'; - path[root_len] = '\0'; + if(path[root_len - 1] != '/') { + path[root_len] = '/'; + root_len++; + path[root_len] = '\0'; + }
ret = _alpm_filelist_resolve_link(files, &i, path, root_len, 0);
On Thu, 14 Feb 2013 22:12:59 +1000 Allan McRae <allan@archlinux.org> wrote:
On 14/02/13 09:54, Andrew Gregory wrote:
Signed-off-by: Andrew Gregory <andrew.gregory.8@gmail.com> ---
OK... I'm tired, so feel free to point out where I am wring here...
AFAIK, realpath will not return a trailing "/". So what is being fixed here? If I take out the unneeded if statement, if am failing to see the off-by-one error...
Realpath does include a "trailing" '/' when root == "/". That's why this was passing the test suite. You can see the error by building a package with a file in /lib and then updating it with the file in /usr/lib.
lib/libalpm/filelist.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/lib/libalpm/filelist.c b/lib/libalpm/filelist.c index f29bb7c..c2ca1ca 100644 --- a/lib/libalpm/filelist.c +++ b/lib/libalpm/filelist.c @@ -198,12 +198,15 @@ int _alpm_filelist_resolve(alpm_handle_t *handle, alpm_filelist_t *files) if(realpath(handle->root, path) == NULL){ return -1; } - root_len = strlen(path) + 1; - if(root_len >= PATH_MAX) { + root_len = strlen(path); + if(root_len + 1 >= PATH_MAX) { return -1; } - path[root_len - 1] = '/'; - path[root_len] = '\0'; + if(path[root_len - 1] != '/') { + path[root_len] = '/'; + root_len++; + path[root_len] = '\0'; + }
ret = _alpm_filelist_resolve_link(files, &i, path, root_len, 0);
On 14/02/13 23:30, Andrew Gregory wrote:
On Thu, 14 Feb 2013 22:12:59 +1000 Allan McRae <allan@archlinux.org> wrote:
On 14/02/13 09:54, Andrew Gregory wrote:
Signed-off-by: Andrew Gregory <andrew.gregory.8@gmail.com> ---
OK... I'm tired, so feel free to point out where I am wring here...
AFAIK, realpath will not return a trailing "/". So what is being fixed here? If I take out the unneeded if statement, if am failing to see the off-by-one error...
Realpath does include a "trailing" '/' when root == "/". That's why this was passing the test suite. You can see the error by building a package with a file in /lib and then updating it with the file in /usr/lib.
I see... It would really help if you added comments like this to the commit message. Ack with a detailed commit message. Allan
participants (2)
-
Allan McRae
-
Andrew Gregory