ggreer / the_silver_searcher

A code-searching tool similar to ack, but faster.
http://geoff.greer.fm/ag/
Apache License 2.0
26.17k stars 1.43k forks source link

ag_scandir() mem over-write for __SVR4 case #188

Open kjk opened 11 years ago

kjk commented 11 years ago
        d = malloc(sizeof(struct dirent) + strlen(entry->d_name));

I'm pretty sure it should be + 1 to account for terminating null.

Wouldn't it be simpler to just strdup d_name and pass that around instead of dirent entries? (per http://www.delorie.com/gnu/docs/glibc/libc_270.html d_name is the only member of dirent that can be relied on anyway).

I'm working on Visual Studio port and I'm using dirent implementation that has dirent as:

struct dirent
{
    char *d_name;
};

which is a 3-rd case that would need to be handled in ag_scandir()

ggreer commented 11 years ago

Wow. How'd you find all these errors? Did you use a static analysis tool or did you painstakingly read the code yourself?

kjk commented 11 years ago

I'm making it compilable with Visual Studio. Those are the things that crashed under Windows, so I've read the relevant code to find a fix.

ggreer commented 11 years ago

I fixed the off-by-one error for Solaris users. Thanks for spotting it.

I return an array of dirents instead of dir names mostly because I started off using scandir() and I wanted ag_scandir() to differ as little as possible. I'm hesitant to return an array of dir names, because I use dirent's d_type in is_symlink and is_directory. Replacing those with stat and lstat on all OSes hurts performance.

is_symlink and is_directory don't reference dirent.d_type on Windows, so a VS build should work once ag_scandir() is fixed.