[pacman-dev] [PATCH 1/2] Remove most usages of strncmp()
The supposed safety blanket of this function is better handled by
explicit length checking and usages of strlen() on known NULL-terminated
strings rather than hoping things fit in a buffer. We also have no need
to fully fill a PATH_MAX length variable with NULLs every time as long
as a single terminating byte is there. Remove usages of it by using
strcpy() or memcpy() as appropriate, after doing length checks via
strlen().
Signed-off-by: Dan McGee
We did some funny stuff here before to allow specifying fully-qualified
package names, such as 'testing/gcc' or 'core/gcc'. However, it was done
by duplicating code, not to mention an early escape if a repository
could not be found for an early target. Something like `pacman -Si
foo/bar core/gcc' would not give expected results, although `pacman -Si
bar gcc' would.
Clean up the code, remove strncpy() usage, and clarify the error
messages a bit.
Signed-off-by: Dan McGee
On 06/07/11 05:20, Dan McGee wrote:
The supposed safety blanket of this function is better handled by explicit length checking and usages of strlen() on known NULL-terminated strings rather than hoping things fit in a buffer. We also have no need to fully fill a PATH_MAX length variable with NULLs every time as long as a single terminating byte is there. Remove usages of it by using strcpy() or memcpy() as appropriate, after doing length checks via strlen().
Signed-off-by: Dan McGee
Ack-by: Allan
On Wed, Jul 6, 2011 at 3:20 AM, Dan McGee
The supposed safety blanket of this function is better handled by explicit length checking and usages of strlen() on known NULL-terminated strings rather than hoping things fit in a buffer. We also have no need to fully fill a PATH_MAX length variable with NULLs every time as long as a single terminating byte is there. Remove usages of it by using strcpy() or memcpy() as appropriate, after doing length checks via strlen().
Signed-off-by: Dan McGee
--- lib/libalpm/handle.c | 2 +- src/pacman/query.c | 17 ++++++++++------- src/pacman/util.c | 11 ++++------- src/util/vercmp.c | 2 +- 4 files changed, 16 insertions(+), 16 deletions(-) diff --git a/lib/libalpm/handle.c b/lib/libalpm/handle.c index 22f3fc8..b48d5b6 100644 --- a/lib/libalpm/handle.c +++ b/lib/libalpm/handle.c @@ -125,9 +124,13 @@ static int query_fileowner(alpm_list_t *targets) * once, then we can just overwrite whatever file was there on the previous * iteration. */ root = alpm_option_get_root(config->handle); - strncpy(path, root, PATH_MAX - 1); - append = path + strlen(path); - max_length = PATH_MAX - (append - path) - 1; + rootlen = strlen(root); + if(rootlen + 1 > PATH_MAX) { + /* we are in trouble here */ + pm_fprintf(stderr, ALPM_LOG_ERROR, _("path too long: %s%s\n"), root, ""); + return 1; + } + strcpy(path, root);
Might be a bit late to the party, but there's potential for a bug to creep in here. If anything is done with "path" before hand, the check becomes incorrect. It should check strlen(path) + rootlen, and is done later on:
@@ -208,11 +211,11 @@ static int query_fileowner(alpm_list_t *targets) continue; }
- if(strlen(pkgfile) > max_length) { + if(rootlen + 1 + strlen(pkgfile) > PATH_MAX) { pm_fprintf(stderr, ALPM_LOG_ERROR, _("path too long: %s%s\n"), root, pkgfile); } /* concatenate our file and the root path */ - strcpy(append, pkgfile); + strcpy(path + rootlen, pkgfile);
pdname = mdirname(path); ppath = resolve_path(pdname);
participants (3)
-
Allan McRae
-
Dan McGee
-
Sebastian Nowicki