Make WordPress Core

Opened 7 years ago

Closed 4 years ago

#44022 closed enhancement (maybelater)

Location information of admin users leaked

Reported by: alicewondermiscreations's profile alicewondermiscreations Owned by:
Milestone: Priority: normal
Severity: normal Version: 4.8
Component: Administration Keywords:
Focuses: administration Cc:

Description

class-wp-community-events.php

That class and what it does really needs to be taken out of core and turned into a plugin. I don't care if it is a plugin that installed by default and even turned on by default, but it needs to be easy to turn off.

https://gist.github.com/AliceWonderMiscreations/b6acab93d03f73ba3b327aaebbf043e1

That turns it off, but that will get undone w/ the next WordPress because it is modifying a core file.

Please turn that feature into a plugin that can easily be turned off by non-technical admins.

Also the class uses unsalted md5 - wouldn't it be better to use one of the site specific salts in the wp-config.php to salt the key created with the md5?

Change History (6)

#1 @iandunn
7 years ago

  • Component changed from General to Administration
  • Focuses administration added
  • Keywords gdpr added
  • Type changed from defect (bug) to enhancement
  • Version set to 4.8

This plugin by @coreymckrill might be what you're looking for:

https://wordpress.org/plugins/community-events-privacy/

Can you explain what you mean by the class "leaking" the location? Do you mean that it's exposed to unauthorized users, or just that it stores the location in the database?

If you think there is an actual security vulnerability, then please don't comment publicly on Trac, since that would expose it to people who want to maliciously exploit it. Instead, please use our HackerOne program.

https://make.wordpress.org/core/handbook/testing/reporting-security-vulnerabilities

If you just don't like the fact that your approximate location is stored, though, then it's fine to continue discussing that in public.

md5() isn't used for security in this case, it's only used as a way to hash all of the input factors to create a unique ID.

#2 @alicewondermiscreations
7 years ago

If you read the class, it uses home_url( '/' ) when building the $request_args array that is sent to the api server. Why it needs to do that is a mystery to me if the API server isn't tracking.

So it is disclosing both the location of the admin user and the domain of the wordpress install to the api server, without the consent of the admin logging in.

I don't believe it is a security issue in the sense that it does not expose any methods for compromising the server to anyone, but it is a privacy concern.

#3 @iandunn
7 years ago

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

home_url() was added in r41605, and is consistent with other requests to api.wordpress.org.

There's an earlier ticket that discusses the privacy issues around those requests in general: #43492. That'd be the best place to add your feedback. IIRC, there are some plugins in the w.org repository which will turn that off as well.

#4 @iandunn
7 years ago

  • Keywords gdpr removed
  • Resolution duplicate deleted
  • Status changed from closed to reopened

Continuing the discussion from ticket:43492#comment:65...

In my experience, the response to the Events Widget has been overwhelmingly positive. It was even featured in last year's State of the Word presentation twice (video, slides). Event attendance is up more than 30% since it was released, which I think is a strong indicator that users are finding it helpful.

The team that worked on it gathered a lot of feedback while it was in the feature plugin phase(1, 2, 3), and that was also very positive. I can only remember one comment questioning whether or not it belonged in Core.

I'm happy to admit there's some selection bias going on there, though, since I've mainly heard feedback from event organizers, and attendees who's learned about events through the widget. It's entirely possible that's there's a large number of users who dislike the widget, and we just haven't heard from them.

If that is the case, I would expect there to be a large number posts on the w.org forums, Trac tickets, comments on blogs like WP Tavern(1, 2), people creating/installing plugins to disable it, etc. Are you aware of anything that would indicate that? If there is some evidence, then it's certainly worth considering.

Given the benefits that the widget has created, though, I personally feel like the evidence would need to be very strong in order to make a compelling case for removing the widget.

If anyone would like to create a plugin that disables the widget, I think it's already possible with existing hooks, depending on exactly what the goal is (prevent network requests, remove events but keep news, remove events and news, etc). If more hooks are needed, though, then that's worth considering too.

#5 @SergeyBiryukov
7 years ago

  • Milestone set to Awaiting Review

#6 @iandunn
4 years ago

  • Milestone Awaiting Review deleted
  • Resolution set to maybelater
  • Status changed from reopened to closed

It seems safe to close this based on the lack of activity, and the availability of plugins to achieve the desired result.

Note: See TracTickets for help on using tickets.