[PATCH] notmuch-new.c infinite recursion symlink bug

[PATCH] notmuch-new.c infinite recursion symlink bug

[Taylor Carpenter]

If a symlink points to . then there will be an infinite recursion.  The included patch fixes that.


--- notmuch-new.c.orig  2011-06-10 00:03:09.000000000 -0500
+++ notmuch-new.c 2011-06-10 02:10:37.000000000 -0500
@@ -233,6 +233,8 @@
     struct stat st;
     notmuch_bool_t is_maildir, new_directory;
     const char **tag;
+    char lpath[PATH_MAX], filepath[PATH_MAX];
+    size_t len;
 
     if (stat (path, &st)) {
  fprintf (stderr, "Error reading directory %s: %s\n",
@@ -296,6 +298,14 @@
   */
  /* XXX: Eventually we'll want more sophistication to let the
   * user specify files to be ignored. */
+
+ if (entry->d_type == DT_LNK) {
+     snprintf(filepath, sizeof(filepath), "%s/%s", path, entry->d_name);
+     if ((len = readlink(filepath, lpath, sizeof(lpath))) > 0)
+         if (strncmp(lpath, ".", len-1) == 0)
+             continue;
+ }
+
  if (strcmp (entry->d_name, ".") == 0 ||
      strcmp (entry->d_name, "..") == 0 ||
      (is_maildir && strcmp (entry->d_name, "tmp") == 0) ||

Re: [PATCH] notmuch-new.c infinite recursion symlink bug

[Taylor Carpenter]

On 06/10/11 at 02:32P, Taylor Carpenter wrote:
> If a symlink points to . then there will be an infinite recursion.  The included patch fixes that.


I did not realize this was needed in the count function as well.  New
patch included that does both.

--- notmuch-new.c.orig  2011-06-10 00:03:09.000000000 -0500
+++ notmuch-new.c 2011-06-10 02:46:18.000000000 -0500
@@ -233,6 +233,8 @@
     struct stat st;
     notmuch_bool_t is_maildir, new_directory;
     const char **tag;
+    char lpath[PATH_MAX], filepath[PATH_MAX];
+    size_t len;
 
     if (stat (path, &st)) {
  fprintf (stderr, "Error reading directory %s: %s\n",
@@ -296,6 +298,14 @@
   */
  /* XXX: Eventually we'll want more sophistication to let the
   * user specify files to be ignored. */
+
+ if (entry->d_type == DT_LNK) {
+     snprintf(filepath, sizeof(filepath), "%s/%s", path, entry->d_name);
+     if ((len = readlink(filepath, lpath, sizeof(lpath))) > 0)
+         if (strncmp(lpath, ".", len-1) == 0)
+             continue;
+ }
+
  if (strcmp (entry->d_name, ".") == 0 ||
      strcmp (entry->d_name, "..") == 0 ||
      (is_maildir && strcmp (entry->d_name, "tmp") == 0) ||
@@ -615,6 +625,8 @@
     struct dirent **fs_entries = NULL;
     int num_fs_entries = scandir (path, &fs_entries, 0, dirent_sort_inode);
     int i = 0;
+    char lpath[PATH_MAX], filepath[PATH_MAX];
+    size_t len;
 
     if (num_fs_entries == -1) {
  fprintf (stderr, "Warning: failed to open directory %s: %s\n",
@@ -633,6 +645,13 @@
   */
  /* XXX: Eventually we'll want more sophistication to let the
   * user specify files to be ignored. */
+ if (entry->d_type == DT_LNK) {
+     snprintf(filepath, sizeof(filepath), "%s/%s", path, entry->d_name);
+     if ((len = readlink(filepath, lpath, sizeof(lpath))) > 0)
+         if (strncmp(lpath, ".", len-1) == 0)
+             continue;
+ }
+
  if (strcmp (entry->d_name, ".") == 0 ||
      strcmp (entry->d_name, "..") == 0 ||
      strcmp (entry->d_name, ".notmuch") == 0)

Re: [PATCH] notmuch-new.c infinite recursion symlink bug

[Daniel Kahn Gillmor]

[-- Attachment #1: Type: text/plain, Size: 668 bytes --]

On 06/10/2011 03:32 AM, Taylor Carpenter wrote:
> If a symlink points to . then there will be an infinite recursion.  The included patch fixes that.

what about a sub-directory that contains a symlink to ".." ?

or a directory that contains both:

 ./a/foo → ../b
 ./b/foo → ../a

or ...

My point is: there are lots of ways to get infinite recursions via
symlinks;  hard-coding a check for one specific way seems like a
sub-optimal approach, because it leaves the other paths still present,
and introduces an unexpected/surprising asymmetry.

I'm not sure what the specific right way is to solve the problem you
identified, though.

	--dkg


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 1030 bytes --]

Re: [PATCH] notmuch-new.c infinite recursion symlink bug

[Austin Clements]

On Tue, Jun 21, 2011 at 8:42 AM, Daniel Kahn Gillmor
<dkg@fifthhorseman.net> wrote:
> My point is: there are lots of ways to get infinite recursions via
> symlinks;  hard-coding a check for one specific way seems like a
> sub-optimal approach, because it leaves the other paths still present,
> and introduces an unexpected/surprising asymmetry.
>
> I'm not sure what the specific right way is to solve the problem you
> identified, though.

A simple solution to this problem would be to record the inode numbers
of each visited directory, probably in a hash table in
add_files_state_t.  At the beginning of add_files_recursive, right
after it stat's the directory, check if st.st_ino is in this hash
table; if it is, return immediately, otherwise add it to the hash
table and proceed as usual.

Alternatively, because of folder search, it might be better to keep a
stack of inode numbers to eliminate loops while retaining notmuch's
current approach of repeatedly indexing mail that's symlinked in
multiple folders.

Re: [PATCH] notmuch-new.c infinite recursion symlink bug

[Carl Worth]

[-- Attachment #1: Type: text/plain, Size: 397 bytes --]

On Tue, 21 Jun 2011 12:01:17 -0400, Austin Clements <amdragon@mit.edu> wrote:
> Alternatively, because of folder search, it might be better to keep a
> stack of inode numbers to eliminate loops while retaining notmuch's
> current approach of repeatedly indexing mail that's symlinked in
> multiple folders.

Yes, that sounds like the right approach.

-Carl

-- 
carl.d.worth@intel.com

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]