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
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
|
From 535f8f816539ba681ef0f12015d2cb587ae61b6d Mon Sep 17 00:00:00 2001
From: Andrew Tridgell <andrew@tridgell.net>
Date: Sat, 23 Nov 2024 15:15:53 +1100
Subject: [PATCH] make --safe-links stricter
when --safe-links is used also reject links where a '../' component is
included in the destination as other than the leading part of the
filename
---
testsuite/safe-links.test | 55 ++++++++++++++++++++++++++++++++++++
testsuite/unsafe-byname.test | 2 +-
util1.c | 26 ++++++++++++++++-
3 files changed, 81 insertions(+), 2 deletions(-)
create mode 100644 testsuite/safe-links.test
diff --git a/testsuite/safe-links.test b/testsuite/safe-links.test
new file mode 100644
index 00000000..6e95a4b9
--- /dev/null
+++ b/testsuite/safe-links.test
@@ -0,0 +1,55 @@
+#!/bin/sh
+
+. "$suitedir/rsync.fns"
+
+test_symlink() {
+ is_a_link "$1" || test_fail "File $1 is not a symlink"
+}
+
+test_regular() {
+ if [ ! -f "$1" ]; then
+ test_fail "File $1 is not regular file or not exists"
+ fi
+}
+
+test_notexist() {
+ if [ -e "$1" ]; then
+ test_fail "File $1 exists"
+ fi
+ if [ -h "$1" ]; then
+ test_fail "File $1 exists as a symlink"
+ fi
+}
+
+cd "$tmpdir"
+
+mkdir from
+
+mkdir "from/safe"
+mkdir "from/unsafe"
+
+mkdir "from/safe/files"
+mkdir "from/safe/links"
+
+touch "from/safe/files/file1"
+touch "from/safe/files/file2"
+touch "from/unsafe/unsafefile"
+
+ln -s ../files/file1 "from/safe/links/"
+ln -s ../files/file2 "from/safe/links/"
+ln -s ../../unsafe/unsafefile "from/safe/links/"
+ln -s a/a/a/../../../unsafe2 "from/safe/links/"
+
+#echo "LISTING FROM"
+#ls -lR from
+
+echo "rsync with relative path and just -a"
+$RSYNC -avv --safe-links from/safe/ to
+
+#echo "LISTING TO"
+#ls -lR to
+
+test_symlink to/links/file1
+test_symlink to/links/file2
+test_notexist to/links/unsafefile
+test_notexist to/links/unsafe2
diff --git a/testsuite/unsafe-byname.test b/testsuite/unsafe-byname.test
index 75e72014..d2e318ef 100644
--- a/testsuite/unsafe-byname.test
+++ b/testsuite/unsafe-byname.test
@@ -40,7 +40,7 @@ test_unsafe ..//../dest from/dir unsafe
test_unsafe .. from/file safe
test_unsafe ../.. from/file unsafe
test_unsafe ..//.. from//file unsafe
-test_unsafe dir/.. from safe
+test_unsafe dir/.. from unsafe
test_unsafe dir/../.. from unsafe
test_unsafe dir/..//.. from unsafe
diff --git a/util1.c b/util1.c
index da50ff1e..f260d398 100644
--- a/util1.c
+++ b/util1.c
@@ -1318,7 +1318,14 @@ int handle_partial_dir(const char *fname, int create)
*
* "src" is the top source directory currently applicable at the level
* of the referenced symlink. This is usually the symlink's full path
- * (including its name), as referenced from the root of the transfer. */
+ * (including its name), as referenced from the root of the transfer.
+ *
+ * NOTE: this also rejects dest names with a .. component in other
+ * than the first component of the name ie. it rejects names such as
+ * a/b/../x/y. This needs to be done as the leading subpaths 'a' or
+ * 'b' could later be replaced with symlinks such as a link to '.'
+ * resulting in the link being transferred now becoming unsafe
+ */
int unsafe_symlink(const char *dest, const char *src)
{
const char *name, *slash;
@@ -1328,6 +1335,23 @@ int unsafe_symlink(const char *dest, const char *src)
if (!dest || !*dest || *dest == '/')
return 1;
+ // reject destinations with /../ in the name other than at the start of the name
+ const char *dest2 = dest;
+ while (strncmp(dest2, "../", 3) == 0) {
+ dest2 += 3;
+ while (*dest2 == '/') {
+ // allow for ..//..///../foo
+ dest2++;
+ }
+ }
+ if (strstr(dest2, "/../"))
+ return 1;
+
+ // reject if the destination ends in /..
+ const size_t dlen = strlen(dest);
+ if (dlen > 3 && strcmp(&dest[dlen-3], "/..") == 0)
+ return 1;
+
/* find out what our safety margin is */
for (name = src; (slash = strchr(name, '/')) != 0; name = slash+1) {
/* ".." segment starts the count over. "." segment is ignored. */
--
2.34.1
|
