Make WordPress Core

Opened 5 years ago

Closed 8 months ago

#44230 closed defect (bug) (wontfix)

Export Personal Data Flaw

Reported by: psycleuk's profile psycleuk Owned by:
Milestone: Priority: normal
Severity: major Version: 4.9.6
Component: Privacy Keywords:
Focuses: Cc:

Description

In testing the new export personal data feature that was introduced in WordPress 4.9.6, i believe i have found a flaw in how it works.

Once a user has confirmed the data request and an admin user clicks the send email option against the request, this creates the zip file in the uploads directory. The zip file is then publicly accessible to anyone that could work out the url for 3 days by default. This seems wrong to me, the zip file should only be accessible to the user who requested it.

As a work around i have had to reduce the expiry time of the zip file, using the wp_privacy_export_expiration filter to be 1 hour. To minimise the window that the file is available and at risk, but still give the requesting user time to access it.

Steps to reproduce:

  • follow the Export Personal Data request process to send the data email
  • use the link in the email to download the zip file, this can be performed by anyone who has/knows the link

Expected result:

  • the wp-personal-data-exports folder should have public access blocked
  • access to the zip file should be through a token (ideally single use) and validated against the requesting user before allowing download

Change History (11)

#1 @johnbillion
5 years ago

  • Keywords close reporter-feedback added

Thanks for the report, @psycleuk.

The zip file is then publicly accessible to anyone that could work out the url for 3 days by default.

The randomised string that gets appended to the file name has a length of 32 characters, and is generated by wp_generate_password() from a pool of 62 possible characters. This gives it an entropy of 32^62, which is a number containing over ninety digits.

Brute force guessing such a filename is therefore out of the question. Are you aware of a means of pre-calculating the filename?

Note also that the directory listing is not readable due to the existence of the index.html file in the directory.

#2 @desrosj
5 years ago

Related: #43546.

For a full breakdown on the thought process behind the file names, please see the explanation in https://core.trac.wordpress.org/ticket/43546#comment:34

#3 @psycleuk
5 years ago

Apologies for not being clear enough in my initial wording - our concerns are that the file has no access control on it.

How guessable the file name is bears no relevance, as secure by obscurity is not secure. Once the file has been created anyone can access it and download it.

As Wordpress is an open source CMS and by default the code is open, it would therefore not be too difficult to reverse engineer how the file is created to build a script to exploit it.

There is also a potential issue with using index.html as a way to block directory viewing of the personal data folder, as any server that is configured to not default to index.html would be exposed. Although, i do appreciate that catering for server configurations is a difficult task. Ideally using .htaccess to block all public access to this folder is preferred, then any developer using nginx only setups would need to copy the rules into the nginx config.

#4 @psycleuk
5 years ago

  • Keywords close removed

Removing the close keyword, as based on my above comment i don't believe this issue should be considered resolved.

Last edited 5 years ago by psycleuk (previous) (diff)

#5 @psycleuk
5 years ago

  • Keywords reporter-feedback removed

Missed removing the reporter-feedback keyword on my previous comments.

Again, i reiterate my point that security by obscurity is not secure. The current implementation has no ACL on who can download the created zip file, which it should be only the user that the data is about and they should have to login to get access to it.

#6 follow-up: @psycleuk
5 years ago

Following up on this again, as there has been no response to concerns about the data file being public and only obscured from general access.

After further review of the process, i believe there is another flaw. A user does not need to log into the site the confirm the request, all they need to do it click the link in the email. The process flow assumes that the person clicking the link in the email will always be the person who triggered the request, but if the users email account is compromised it may not be the case.

The current process flow would allow as user to request data from a WordPress site without ever logging into the site to confirm who they are, all they would need access to is the email with the confirmation link.

Given that the data being requested is about a user of the site and will therefore have an account on the site, surely the safest process to ensure data security is to have the user log into their account at each step to confirm they are the correct user.

#7 in reply to: ↑ 6 @xkon
5 years ago

  • Keywords close added

Replying to psycleuk:

Following up on this again, as there has been no response to concerns about the data file being public and only obscured from general access.

Let's say you know the user's email that you are after. You'd still have to find the time-window that the zip is available + the hash.

The only way to have everything is to actually have access to the user's e-mail. And then there's seriously nothing 'we' can do about it.

After further review of the process, i believe there is another flaw. A user does not need to log into the site the confirm the request, all they need to do it click the link in the email. The process flow assumes that the person clicking the link in the email will always be the person who triggered the request, but if the users email account is compromised it may not be the case.

The current process flow would allow as user to request data from a WordPress site without ever logging into the site to confirm who they are, all they would need access to is the email with the confirmation link.

Given that the data being requested is about a user of the site and will therefore have an account on the site, surely the safest process to ensure data security is to have the user log into their account at each step to confirm they are the correct user.

A user does not need to log in to a website because he might not have an account. For example comments don't require user registration, so there's no need for a login to retrieve that data, you just need the e-mail address that you made the comments with. There are plenty of use-cases that will never require actual 'registration' or even actual 'access' to an application/website that collects data.

If a users e-mail is compromised as you say, then the attacker would probably have access on all of the subjects data already. The users e-mail security isn't our problem I believe nor we can do anything about it. On the other hand most companies that I know handling important data, do require extra validation measures i.e. by phone as well first before even they reach the point of e-mail validation (if they don't want to skip it) etc.

#8 @psycleuk
5 years ago

I take your point about users that can comment and not have a user account, so forcing a login step on this action would not be possible.

Just because there is nothing that you can do about is a users email account has been compromised, does not mean you can be lax with security of users data. The point is the file should not be in the public domain, which it is, regardless of how hard it is to find.

If adding a login control to access the file is not possible, it should at least only be accessible through the use of a single use token. The zip should not be downloadable directly as you can not verify who downloaded it, you can only assume because the file is obscured that it was the correct user. I don't see that as a good enough implementation when it comes to the security of user data. The only way this process could then be exploited is to have access to the users email account, which i am aware is out of your control, but at least WordPress has done everything within it's control to ensure data security.

Also, with relation to https://blog.ripstech.com/2018/wordpress-file-delete-to-code-execution/ that was posted on the 26th June, there is a potential vulnerability where the index.html could be deleted, leaving the entire wp-personal-data-exports folder publicly traversable.

#9 @psycleuk
5 years ago

  • Keywords close removed

This ticket was mentioned in Slack in #core-privacy by garrett-eclipse. View the logs.


4 years ago

#11 @iandunn
8 months ago

  • Milestone Awaiting Review deleted
  • Resolution set to wontfix
  • Status changed from new to closed

Calling the brute-force-resistant file names security-through-obscurity seems analogous to saying that HTTPS traffic on a WiFi network is open to anyone. It's true that technically anybody can read it, but in practice the encryption makes it secure enough for common use.

Mitigations have to adapt to fit the circumstances, and a formal ACL would add extra complexity and degrade UX without adding any tangible benefit. WP is designed for the majority, but the beauty of the plugin system is that anybody can write something to add additional security here for environments that require it.

If you feel like this affects the majority, then I'd challenge you to produce a PoC that demonstrates it in the wild. If you can, then we could definitely consider adding stronger measures! If you are able to, please share it privately on https://hackerone.com/wordpress, so we can add those measures before the PoC becomes public.

The file-deletion vulnerability you mentioned was fixed in 4.9.7. The index.html file was renamed to index.php in #52299 to protect against the configurations you mentioned.

I'm going to close this since it seems like there's a consensus among component maintainers and committers. That just means there isn't anything actionable right now, though. We can continue discussing it, and could reopen this if a compelling argument is made or new information is presented.

Note: See TracTickets for help on using tickets.