You are not logged in.

RemoveFileOrDir in tools.c

Dear visitor, welcome to VDR Portal. If this is your first visit here, please read the Help. It explains in detail how this page works. To use all features of this page, you should consider registering. Please use the registration form, to register here or read more information about the registration process. If you are already registered, please login here.

atinar

Beginner

  • "atinar" started this thread

Posts: 2

  • Send private message

1

Sunday, June 20th 2010, 1:32am

RemoveFileOrDir in tools.c

Hello,
First of all, excuse me for posting in English, but I've not seen nothing related to patches in international forums. Feel free to reply in German.

In tools.c, when removing a file that is a symlink, the buffer size allocated for the target file path is twice the size of the link path. Under certain circumstances the supposition this is enough is wrong.

In countries like mine (allow me to omit which one it is...) there are many TV stations which have serious problems with the quality of the EPG they provide. In particular, they don't seem to understand the difference between the "short description" and "description" fields and are prone to fill the first one with the second one. If you are recording a series episode you may end with a filename which contains as much of the description as possible. When you change the name of the recording for a more reasonable one, the problem is served.

I think that a call to stat, should be done instead. This patch is for an old vdr, but I think with minor changes it still applies:

Source code

 
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46

--- VDR-ori/tools.c     2010-06-20 00:37:37.000000000 +0200
+++ VDR-bui/tools.c     2010-06-20 01:01:18.000000000 +0200
@@ -374,22 +374,29 @@
                     char *buffer;
                     asprintf(&buffer, "%s/%s", FileName, e->d_name);
                     if (FollowSymlinks) {
-                       int size = strlen(buffer) * 2; // should be large enough
-                       char *l = MALLOC(char, size);
-                       int n = readlink(buffer, l, size);
-                       if (n < 0) {
-                          if (errno != EINVAL)
-                             LOG_ERROR_STR(buffer);
+                       struct stat st2;
+                       if (stat(buffer, &st2) == 0) {
+                          int size = st2.st_size + 1;
+                          char *l = MALLOC(char, size);
+                          int n = readlink(buffer, l, st2.st_size);
+                          if (n < 0) {
+                              if (errno != EINVAL)
+                                  LOG_ERROR_STR(buffer);
+                              }
+                          else if (n < size) {
+                              l[n] = 0;
+                              dsyslog("removing %s", l);
+                              if (remove(l) < 0)
+                                  LOG_ERROR_STR(l);
+                              }
+                          else
+                              esyslog("ERROR: symlink name length (%d) exceeded anticipated buffer size (%d)", n, size);
+                          free(l);
                           }
-                       else if (n < size) {
-                          l[n] = 0;
-                          dsyslog("removing %s", l);
-                          if (remove(l) < 0)
-                             LOG_ERROR_STR(l);
+                       else if (errno != ENOENT) {
+                          LOG_ERROR_STR(FileName);
+                          return false;
                           }
-                       else
-                          esyslog("ERROR: symlink name length (%d) exceeded anticipated buffer size (%d)", n, size);
-                       free(l);
                        }
                     dsyslog("removing %s", buffer);
                     if (remove(buffer) < 0)

Urig

Professional

Posts: 1,214

Location: Kassel

  • Send private message

2

Saturday, June 26th 2010, 2:52pm

I agree that this is a bug, though it does not apply to vanilla VDR, since VDR itself cannot rename a recording, and the symlink always has the same name as the recording itself. Unless you have very different path lengths to your recording directories.

However, IMHO this patch misses a test whether the stated file actually IS an symbolic link. If the name refers to a real file, a filename buffer of the size of the file will be allocated, which may be several gigabytes. Just add a test to S_ISLNK(st2.st_mode).


Cheers,

Udo

atinar

Beginner

  • "atinar" started this thread

Posts: 2

  • Send private message

3

Sunday, June 27th 2010, 2:42pm

You're right Udo, I didn't realize the condition of buffer being a symlink was not tested before calling readlink . I'm glad I posted this here before using it... ;-).

Thanks,
atinar

kls

Master

Posts: 2,166

Location: Mettenheim

  • Send private message

4

Sunday, August 29th 2010, 4:59pm

Please test the following patch, which also includes Udo's suggestion (I hope I've put it at the right place ;-):

Source code

 
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48

--- tools.c     2010/02/28 13:31:46     2.7
+++ tools.c     2010/08/29 14:50:28
@@ -367,22 +367,31 @@
                  if (strcmp(e->d_name, ".") && strcmp(e->d_name, "..")) {
                     cString buffer = AddDirectory(FileName, e->d_name);
                     if (FollowSymlinks) {
-                       int size = strlen(buffer) * 2; // should be large enough
-                       char *l = MALLOC(char, size);
-                       int n = readlink(buffer, l, size);
-                       if (n < 0) {
-                          if (errno != EINVAL)
-                             LOG_ERROR_STR(*buffer);
+                       struct stat st2;
+                       if (stat(buffer, &st2) == 0) {
+                          if (S_ISLNK(st2.st_mode)) {
+                             int size = st2.st_size + 1;
+                             char *l = MALLOC(char, size);
+                             int n = readlink(buffer, l, st2.st_size);
+                             if (n < 0) {
+                                if (errno != EINVAL)
+                                   LOG_ERROR_STR(*buffer);
+                                }
+                             else if (n < size) {
+                                l[n] = 0;
+                                dsyslog("removing %s", l);
+                                if (remove(l) < 0)
+                                   LOG_ERROR_STR(l);
+                                }
+                             else
+                                esyslog("ERROR: symlink name length (%d) exceeded anticipated buffer size (%d)", n, size);
+                             free(l);
+                             }
                           }
-                       else if (n < size) {
-                          l[n] = 0;
-                          dsyslog("removing %s", l);
-                          if (remove(l) < 0)
-                             LOG_ERROR_STR(l);
+                       else if (errno != ENOENT) {
+                          LOG_ERROR_STR(FileName);
+                          return false;
                           }
-                       else
-                          esyslog("ERROR: symlink name length (%d) exceeded anticipated buffer size (%d)", n, size);
-                       free(l);
                        }
                     dsyslog("removing %s", *buffer);
                     if (remove(buffer) < 0)


BTW: I would need your real name and email address if you want to be mentioned in VDR's CONTRIBUTORS file.

Klaus
Gib CI+/HD+ keine Chance! Lasst diese Pest am ausgestreckten Arm verhungern!
Wer für sowas bezahlt macht sich zum Totengräber von Projekten wie VDR!
Die Wahrheit ueber HD Plus
CI-Plus -- Das trojanische Pferd im Wohnzimmer