Skip to content

Commit 7d7aabb

Browse files
DeepDiver1975claude
DeepDiver1975
and
claude
committed
fix(comments): prevent IDOR in WebDAV comments API (#41558)
* test(comments): stub objectType/objectId in EntityCollection happy-path tests Signed-off-by: Thomas Müller <1005065+DeepDiver1975@users.noreply.github.com> * test(comments): add failing IDOR regression tests for EntityCollection Signed-off-by: Thomas Müller <1005065+DeepDiver1975@users.noreply.github.com> * fix(comments): prevent IDOR in WebDAV comments API by checking comment ownership An authenticated user could PROPFIND/DELETE/PROPPATCH any comment by supplying an arbitrary comment_id paired with any file_id they own. EntityCollection::getChild() and childExists() now verify that the fetched comment's objectType and objectId match the collection's own entity type and file ID before returning or confirming the node. Fixes OC10-53 Signed-off-by: Thomas Müller <1005065+DeepDiver1975@users.noreply.github.com> * docs: add changelog entry for OC10-53 IDOR fix in WebDAV comments API Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Thomas Müller <1005065+DeepDiver1975@users.noreply.github.com> --------- Signed-off-by: Thomas Müller <1005065+DeepDiver1975@users.noreply.github.com> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent b567384 commit 7d7aabb

3 files changed

Lines changed: 83 additions & 3 deletions

File tree

‎apps/comments/lib/Dav/EntityCollection.php‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,10 @@ public function getId() {
9898
public function getChild($name) {
9999
try {
100100
$comment = $this->commentsManager->get($name);
101+
// objectType/objectId identify the entity (e.g. file) the comment is attached to
102+
if ($comment->getObjectType() !== $this->name || $comment->getObjectId() !== $this->id) {
103+
throw new NotFound();
104+
}
101105
return new CommentNode(
102106
$this->commentsManager,
103107
$comment,
@@ -151,8 +155,10 @@ public function findChildren($limit = 0, $offset = 0, \DateTime $datetime = null
151155
*/
152156
public function childExists($name) {
153157
try {
154-
$this->commentsManager->get($name);
155-
return true;
158+
$comment = $this->commentsManager->get($name);
159+
// objectType/objectId identify the entity (e.g. file) the comment is attached to
160+
return $comment->getObjectType() === $this->name
161+
&& $comment->getObjectId() === $this->id;
156162
} catch (NotFoundException $e) {
157163
return false;
158164
}

‎apps/comments/tests/unit/Dav/EntityCollectionTest.php‎

Lines changed: 70 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,10 +70,14 @@ public function testGetId(): void {
7070
}
7171

7272
public function testGetChild(): void {
73+
$comment = $this->createMock(IComment::class);
74+
$comment->method('getObjectType')->willReturn('files');
75+
$comment->method('getObjectId')->willReturn('19');
76+
7377
$this->commentsManager->expects($this->once())
7478
->method('get')
7579
->with('55')
76-
->will($this->returnValue($this->createMock(IComment::class)));
80+
->willReturn($comment);
7781

7882
$node = $this->collection->getChild('55');
7983
$this->assertInstanceOf(\OCA\Comments\Dav\CommentNode::class, $node);
@@ -118,6 +122,15 @@ public function testFindChildren(): void {
118122
}
119123

120124
public function testChildExistsTrue(): void {
125+
$comment = $this->createMock(IComment::class);
126+
$comment->method('getObjectType')->willReturn('files');
127+
$comment->method('getObjectId')->willReturn('19');
128+
129+
$this->commentsManager->expects($this->once())
130+
->method('get')
131+
->with('44')
132+
->willReturn($comment);
133+
121134
$this->assertTrue($this->collection->childExists('44'));
122135
}
123136

@@ -129,4 +142,60 @@ public function testChildExistsFalse(): void {
129142

130143
$this->assertFalse($this->collection->childExists('44'));
131144
}
145+
146+
public function testGetChildWrongFile(): void {
147+
$this->expectException(\Sabre\DAV\Exception\NotFound::class);
148+
149+
$comment = $this->createMock(IComment::class);
150+
$comment->method('getObjectType')->willReturn('files');
151+
$comment->method('getObjectId')->willReturn('999'); // different file
152+
153+
$this->commentsManager->expects($this->once())
154+
->method('get')
155+
->with('55')
156+
->willReturn($comment);
157+
158+
$this->collection->getChild('55');
159+
}
160+
161+
public function testGetChildWrongObjectType(): void {
162+
$this->expectException(\Sabre\DAV\Exception\NotFound::class);
163+
164+
$comment = $this->createMock(IComment::class);
165+
$comment->method('getObjectType')->willReturn('albums'); // wrong type
166+
$comment->method('getObjectId')->willReturn('19');
167+
168+
$this->commentsManager->expects($this->once())
169+
->method('get')
170+
->with('55')
171+
->willReturn($comment);
172+
173+
$this->collection->getChild('55');
174+
}
175+
176+
public function testChildExistsWrongFile(): void {
177+
$comment = $this->createMock(IComment::class);
178+
$comment->method('getObjectType')->willReturn('files');
179+
$comment->method('getObjectId')->willReturn('999'); // different file
180+
181+
$this->commentsManager->expects($this->once())
182+
->method('get')
183+
->with('44')
184+
->willReturn($comment);
185+
186+
$this->assertFalse($this->collection->childExists('44'));
187+
}
188+
189+
public function testChildExistsWrongObjectType(): void {
190+
$comment = $this->createMock(IComment::class);
191+
$comment->method('getObjectType')->willReturn('albums'); // wrong type
192+
$comment->method('getObjectId')->willReturn('19');
193+
194+
$this->commentsManager->expects($this->once())
195+
->method('get')
196+
->with('44')
197+
->willReturn($comment);
198+
199+
$this->assertFalse($this->collection->childExists('44'));
200+
}
132201
}

‎changelog/unreleased/41558‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
Fix: Prevent IDOR in WebDAV comments API
2+
3+
Authenticated users could read, edit, or delete comments on files they have no access to by supplying an arbitrary comment ID in the WebDAV comments endpoint. The fix verifies that a requested comment belongs to the file in the URL before returning it.
4+
5+
https://github.com/owncloud/core/pull/41558

0 commit comments

Comments
 (0)