System address book registered in contacts manager may lead to data leakage

[ChristophWurst]

As discovered in the mail app, the dav app registers the system users address book for the contacts manager. If any app queries this manager, all personal and system-wide contacts can be retrieved, even if sharing options are configured to not suggest any other system users.

Ref nextcloud/mail#848 (comment)

server/lib/private/ContactsManager.php

Lines 41 to 54 in 7b4e51d

	public function search($pattern, $searchProperties = array(), $options = array()) {
	$this->loadAddressBooks();
	$result = array();
	foreach($this->addressBooks as $addressBook) {
	$r = $addressBook->search($pattern, $searchProperties, $options);
	$contacts = array();
	foreach($r as $c){
	$c['addressbook-key'] = $addressBook->getKey();
	$contacts[] = $c;
	}
	$result = array_merge($result, $contacts);
	}
	return $result;

server/apps/dav/lib/CardDAV/ContactsManager.php

Lines 55 to 59 in 7b4e51d

	public function setupContactsProvider(IManager $cm, $userId, IURLGenerator $urlGenerator) {
	$addressBooks = $this->backend->getAddressBooksForUser("principals/users/$userId");
	$this->register($cm, $addressBooks, $urlGenerator);
	$this->setupSystemContactsProvider($cm, $urlGenerator);
	}

cc @rullzer

[Peterede]

This also occurs in the Calendar app when adding attendees to a meeting

[daspuppy]

This having the 'security' tag for obvious reasons, and there not having been any activity on this for several months, I am wondering: Is this being worked on?

[rullzer]

Right. This fell off my radar.

I think the address book is doing what is was intended to do. You are not sharing files but want to send e-mails and if people have their e-mail address set to be shared within this instance it should show up.

Of course one could argue that if the admin sets sharing to disabled to suggest other users this should be disabled as well.

I guess the quick fix here would be to just not register the system address book if suggestion are disabled.

[rullzer]

As discussed with @ChristophWurst

This is fixed in the mail app now. Apps should use the IContactsStore to obtain contacts. I'll see if disable enumeration as well on the dav