Skip to content

Commit f1ea284

Browse files
provokateurinprovokateurin
authored
Merge pull request #50052 from nextcloud/fix/files_sharing/harden-api
2 parents 9c717aa + 1e28657 commit f1ea284

File tree

6 files changed

+65
-44
lines changed

6 files changed

+65
-44
lines changed

‎apps/files_sharing/lib/Controller/DeletedShareAPIController.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ public function __construct(
3838
string $appName,
3939
IRequest $request,
4040
private ShareManager $shareManager,
41-
private string $userId,
41+
private ?string $userId,
4242
private IUserManager $userManager,
4343
private IGroupManager $groupManager,
4444
private IRootFolder $rootFolder,

‎apps/files_sharing/lib/Controller/ShareesAPIController.php‎

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -66,19 +66,10 @@ class ShareesAPIController extends OCSController {
6666

6767
protected $reachedEndFor = [];
6868

69-
/**
70-
* @param string $UserId
71-
* @param string $appName
72-
* @param IRequest $request
73-
* @param IConfig $config
74-
* @param IURLGenerator $urlGenerator
75-
* @param IManager $shareManager
76-
* @param ISearch $collaboratorSearch
77-
*/
7869
public function __construct(
7970
string $appName,
8071
IRequest $request,
81-
protected string $userId,
72+
protected ?string $userId,
8273
protected IConfig $config,
8374
protected IURLGenerator $urlGenerator,
8475
protected IManager $shareManager,

‎apps/files_sharing/lib/External/Manager.php‎

Lines changed: 5 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -160,13 +160,7 @@ private function writeShareToDb($remote, $token, $password, $name, $owner, $user
160160
$query->execute([$remote, $token, $password, $name, $owner, $user, $mountPoint, $hash, $accepted, $remoteId, $parent, $shareType]);
161161
}
162162

163-
/**
164-
* get share
165-
*
166-
* @param int $id share id
167-
* @return mixed share of false
168-
*/
169-
private function fetchShare($id) {
163+
private function fetchShare(int $id): array|false {
170164
$getShare = $this->connection->prepare('
171165
SELECT `id`, `remote`, `remote_id`, `share_token`, `name`, `owner`, `user`, `mountpoint`, `accepted`, `parent`, `share_type`, `password`, `mountpoint_hash`
172166
FROM `*PREFIX*share_external`
@@ -208,15 +202,12 @@ private function fetchUserShare($parentId, $uid) {
208202
return null;
209203
}
210204

211-
/**
212-
* get share
213-
*
214-
* @param int $id share id
215-
* @return mixed share of false
216-
*/
217205
public function getShare(int $id, ?string $user = null): array|false {
218206
$user = $user ?? $this->uid;
219207
$share = $this->fetchShare($id);
208+
if ($share === false) {
209+
return false;
210+
}
220211

221212
// check if the user is allowed to access it
222213
if ($this->canAccessShare($share, $user)) {
@@ -256,7 +247,7 @@ private function canAccessShare(array $share, string $user): bool {
256247
&& $share['user'] === $user) {
257248
return true;
258249
}
259-
250+
260251
// If the share is a group share, check if the user is in the group
261252
if ((int)$share['share_type'] === IShare::TYPE_GROUP) {
262253
$parentId = (int)$share['parent'];

‎apps/files_sharing/lib/ResponseDefinitions.php‎

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,6 @@
9999
* }
100100
*
101101
* @psalm-type Files_SharingSharee = array{
102-
* count: int|null,
103102
* label: string,
104103
* }
105104
*
@@ -108,6 +107,14 @@
108107
* shareWith: string,
109108
* }
110109
*
110+
* @psalm-type Files_SharingShareeGroup = Files_SharingSharee&array{
111+
* value: Files_SharingShareeValue,
112+
* }
113+
*
114+
* @psalm-type Files_SharingShareeRoom = Files_SharingSharee&array{
115+
* value: Files_SharingShareeValue,
116+
* }
117+
*
111118
* @psalm-type Files_SharingShareeUser = Files_SharingSharee&array{
112119
* subline: string,
113120
* icon: string,
@@ -180,33 +187,33 @@
180187
* exact: array{
181188
* circles: list<Files_SharingShareeCircle>,
182189
* emails: list<Files_SharingShareeEmail>,
183-
* groups: list<Files_SharingSharee>,
190+
* groups: list<Files_SharingShareeGroup>,
184191
* remote_groups: list<Files_SharingShareeRemoteGroup>,
185192
* remotes: list<Files_SharingShareeRemote>,
186-
* rooms: list<Files_SharingSharee>,
193+
* rooms: list<Files_SharingShareeRoom>,
187194
* users: list<Files_SharingShareeUser>,
188195
* },
189196
* circles: list<Files_SharingShareeCircle>,
190197
* emails: list<Files_SharingShareeEmail>,
191-
* groups: list<Files_SharingSharee>,
198+
* groups: list<Files_SharingShareeGroup>,
192199
* lookup: list<Files_SharingShareeLookup>,
193200
* remote_groups: list<Files_SharingShareeRemoteGroup>,
194201
* remotes: list<Files_SharingShareeRemote>,
195-
* rooms: list<Files_SharingSharee>,
202+
* rooms: list<Files_SharingShareeRoom>,
196203
* users: list<Files_SharingShareeUser>,
197204
* lookupEnabled: bool,
198205
* }
199206
*
200207
* @psalm-type Files_SharingShareesRecommendedResult = array{
201208
* exact: array{
202209
* emails: list<Files_SharingShareeEmail>,
203-
* groups: list<Files_SharingSharee>,
210+
* groups: list<Files_SharingShareeGroup>,
204211
* remote_groups: list<Files_SharingShareeRemoteGroup>,
205212
* remotes: list<Files_SharingShareeRemote>,
206213
* users: list<Files_SharingShareeUser>,
207214
* },
208215
* emails: list<Files_SharingShareeEmail>,
209-
* groups: list<Files_SharingSharee>,
216+
* groups: list<Files_SharingShareeGroup>,
210217
* remote_groups: list<Files_SharingShareeRemoteGroup>,
211218
* remotes: list<Files_SharingShareeRemote>,
212219
* users: list<Files_SharingShareeUser>,

‎apps/files_sharing/openapi.json‎

Lines changed: 42 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -766,15 +766,9 @@
766766
"Sharee": {
767767
"type": "object",
768768
"required": [
769-
"count",
770769
"label"
771770
],
772771
"properties": {
773-
"count": {
774-
"type": "integer",
775-
"format": "int64",
776-
"nullable": true
777-
},
778772
"label": {
779773
"type": "string"
780774
}
@@ -851,6 +845,24 @@
851845
}
852846
]
853847
},
848+
"ShareeGroup": {
849+
"allOf": [
850+
{
851+
"$ref": "#/components/schemas/Sharee"
852+
},
853+
{
854+
"type": "object",
855+
"required": [
856+
"value"
857+
],
858+
"properties": {
859+
"value": {
860+
"$ref": "#/components/schemas/ShareeValue"
861+
}
862+
}
863+
}
864+
]
865+
},
854866
"ShareeLookup": {
855867
"allOf": [
856868
{
@@ -1027,6 +1039,24 @@
10271039
}
10281040
]
10291041
},
1042+
"ShareeRoom": {
1043+
"allOf": [
1044+
{
1045+
"$ref": "#/components/schemas/Sharee"
1046+
},
1047+
{
1048+
"type": "object",
1049+
"required": [
1050+
"value"
1051+
],
1052+
"properties": {
1053+
"value": {
1054+
"$ref": "#/components/schemas/ShareeValue"
1055+
}
1056+
}
1057+
}
1058+
]
1059+
},
10301060
"ShareeUser": {
10311061
"allOf": [
10321062
{
@@ -1129,7 +1159,7 @@
11291159
"groups": {
11301160
"type": "array",
11311161
"items": {
1132-
"$ref": "#/components/schemas/Sharee"
1162+
"$ref": "#/components/schemas/ShareeGroup"
11331163
}
11341164
},
11351165
"remote_groups": {
@@ -1161,7 +1191,7 @@
11611191
"groups": {
11621192
"type": "array",
11631193
"items": {
1164-
"$ref": "#/components/schemas/Sharee"
1194+
"$ref": "#/components/schemas/ShareeGroup"
11651195
}
11661196
},
11671197
"remote_groups": {
@@ -1226,7 +1256,7 @@
12261256
"groups": {
12271257
"type": "array",
12281258
"items": {
1229-
"$ref": "#/components/schemas/Sharee"
1259+
"$ref": "#/components/schemas/ShareeGroup"
12301260
}
12311261
},
12321262
"remote_groups": {
@@ -1244,7 +1274,7 @@
12441274
"rooms": {
12451275
"type": "array",
12461276
"items": {
1247-
"$ref": "#/components/schemas/Sharee"
1277+
"$ref": "#/components/schemas/ShareeRoom"
12481278
}
12491279
},
12501280
"users": {
@@ -1270,7 +1300,7 @@
12701300
"groups": {
12711301
"type": "array",
12721302
"items": {
1273-
"$ref": "#/components/schemas/Sharee"
1303+
"$ref": "#/components/schemas/ShareeGroup"
12741304
}
12751305
},
12761306
"lookup": {
@@ -1294,7 +1324,7 @@
12941324
"rooms": {
12951325
"type": "array",
12961326
"items": {
1297-
"$ref": "#/components/schemas/Sharee"
1327+
"$ref": "#/components/schemas/ShareeRoom"
12981328
}
12991329
},
13001330
"users": {

‎lib/private/Files/Utils/PathHelper.php‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,8 @@ public static function normalizePath(string $path): string {
3737
if ($path === '' or $path === '/') {
3838
return '/';
3939
}
40+
// No null bytes
41+
$path = str_replace(chr(0), '', $path);
4042
//no windows style slashes
4143
$path = str_replace('\\', '/', $path);
4244
//add leading slash

0 commit comments

Comments
 (0)