Fix a buffer overrun and subsequent crash that may occur when ag is given

an absolute path as root of the tree to search: it's looking for a trailing
slash that isn't always there.

Fix by Allen Wild, https://github.com/ggreer/the_silver_searcher/pull/1040/

ok awolk, sthen, jca
This commit is contained in:
tb 2017-01-05 13:50:49 +00:00
parent 01d4b9d4df
commit c5c8e787e5
5 changed files with 125 additions and 1 deletions

View File

@ -1,8 +1,9 @@
# $OpenBSD: Makefile,v 1.23 2016/12/15 14:16:38 fcambus Exp $
# $OpenBSD: Makefile,v 1.24 2017/01/05 13:50:49 tb Exp $
COMMENT = code searching tool, with a focus on speed (ag)
DISTNAME = the_silver_searcher-1.0.2
REVISION = 0
CATEGORIES = textproc
HOMEPAGE = https://github.com/ggreer/the_silver_searcher

View File

@ -0,0 +1,24 @@
$OpenBSD: patch-src_ignore_c,v 1.1 2017/01/05 13:50:49 tb Exp $
Fix heap overflow.
https://github.com/ggreer/the_silver_searcher/pull/1040/
--- src/ignore.c.orig Sat Dec 3 21:46:47 2016
+++ src/ignore.c Tue Jan 3 22:16:05 2017
@@ -295,15 +295,7 @@ int filename_filter(const char *path, const struct dir
}
scandir_baton_t *scandir_baton = (scandir_baton_t *)baton;
- const char *base_path = scandir_baton->base_path;
- const size_t base_path_len = scandir_baton->base_path_len;
- const char *path_start = path;
-
- for (i = 0; base_path[i] == path[i] && i < base_path_len; i++) {
- /* base_path always ends with "/\0" while path doesn't, so this is safe */
- path_start = path + i + 2;
- }
- log_debug("path_start %s filename %s", path_start, filename);
+ const char *path_start = scandir_baton->path_start;
const char *extension = strchr(filename, '.');
if (extension) {

View File

@ -0,0 +1,46 @@
$OpenBSD: patch-src_options_c,v 1.1 2017/01/05 13:50:49 tb Exp $
Fix heap overflow.
https://github.com/ggreer/the_silver_searcher/pull/1040/
--- src/options.c.orig Sat Dec 3 22:54:24 2016
+++ src/options.c Tue Jan 3 22:16:05 2017
@@ -199,6 +199,7 @@ void parse_options(int argc, char **argv, char **base_
int ch;
size_t i;
int path_len = 0;
+ int base_path_len = 0;
int useless = 0;
int group = 1;
int help = 0;
@@ -775,6 +776,7 @@ void parse_options(int argc, char **argv, char **base_
}
char *path = NULL;
+ char *base_path = NULL;
#ifdef PATH_MAX
char *tmp = NULL;
#endif
@@ -792,10 +794,20 @@ void parse_options(int argc, char **argv, char **base_
(*paths)[i] = path;
#ifdef PATH_MAX
tmp = ag_malloc(PATH_MAX);
- (*base_paths)[i] = realpath(path, tmp);
+ base_path = realpath(path, tmp);
#else
- (*base_paths)[i] = realpath(path, NULL);
+ base_path = realpath(path, NULL);
#endif
+ if (base_path) {
+ base_path_len = strlen(base_path);
+ /* add trailing slash */
+ if (base_path_len > 1 && base_path[base_path_len - 1] != '/') {
+ base_path = ag_realloc(base_path, base_path_len + 2);
+ base_path[base_path_len] = '/';
+ base_path[base_path_len + 1] = '\0';
+ }
+ }
+ (*base_paths)[i] = base_path;
}
/* Make sure we search these paths instead of stdin. */
opts.search_stream = 0;

View File

@ -0,0 +1,15 @@
$OpenBSD: patch-src_scandir_h,v 1.1 2017/01/05 13:50:49 tb Exp $
Fix heap overflow.
https://github.com/ggreer/the_silver_searcher/pull/1040/
--- src/scandir.h.orig Mon Nov 28 06:07:11 2016
+++ src/scandir.h Tue Jan 3 22:16:05 2017
@@ -7,6 +7,7 @@ typedef struct {
const ignores *ig;
const char *base_path;
size_t base_path_len;
+ const char *path_start;
} scandir_baton_t;
typedef int (*filter_fp)(const char *path, const struct dirent *, void *);

View File

@ -0,0 +1,38 @@
$OpenBSD: patch-src_search_c,v 1.1 2017/01/05 13:50:49 tb Exp $
Fix heap overflow.
https://github.com/ggreer/the_silver_searcher/pull/1040/
--- src/search.c.orig Sat Dec 3 23:40:19 2016
+++ src/search.c Tue Jan 3 22:16:05 2017
@@ -463,6 +463,8 @@ void search_dir(ignores *ig, const char *base_path, co
struct dirent *dir = NULL;
scandir_baton_t scandir_baton;
int results = 0;
+ size_t base_path_len = 0;
+ const char *path_start = path;
char *dir_full_path = NULL;
const char *ignore_file = NULL;
@@ -486,9 +488,20 @@ void search_dir(ignores *ig, const char *base_path, co
dir_full_path = NULL;
}
+ /* path_start is the part of path that isn't in base_path
+ * base_path will have a trailing '/' because we put it there in parse_options
+ */
+ base_path_len = base_path ? strlen(base_path) : 0;
+ for (i = 0; ((size_t)i < base_path_len) && (path[i]) && (base_path[i] == path[i]); i++) {
+ path_start = path + i + 1;
+ }
+ log_debug("search_dir: path is '%s', base_path is '%s', path_start is '%s'", path, base_path, path_start);
+
scandir_baton.ig = ig;
scandir_baton.base_path = base_path;
- scandir_baton.base_path_len = base_path ? strlen(base_path) : 0;
+ scandir_baton.base_path_len = base_path_len;
+ scandir_baton.path_start = path_start;
+
results = ag_scandir(path, &dir_list, &filename_filter, &scandir_baton);
if (results == 0) {
log_debug("No results found in directory %s", path);