Page MenuHomePhabricator

Investigate: Which extensions save IP addresses
Open, Needs TriagePublic

Description

See also T356430: Investigate: Where does MW core save IP addresses.

This is spun out from T336187: [S] Investigate: Creating temp user on non-EditPage actions, as explained in T336187#9341749:

To guard against accidentally creating an IP actor (because IP actors will now pass permissions checks), do the following:

  • [...]
  • Look for workflows that save the IP address other than through the actor table (e.g. like Flow: T345578#9290889)
Background

If temporary accounts are enabled, the ActorStore throws an error on attempting to create an actor with an IP address. This means that a revision authored by an IP actor won't be able to be saved, and more generally any action that creates an IP actor should not be possible.

We found an exception to this in the Flow extension. Flow saves the comment, along with the IP address (as the author) in a local Flow table. It then attempts to create the IP actor in core but fails - but the damage is already done. The IP address saved in the Flow table will appear in the UI. (More details in T345578#9290889.)

In this task we want to look for other examples of extensions that save IP addresses.

What to investigate
  • Which extensions save IP addresses?
  • Do these cases need fixing, and if so how (or who can tell us how)?

Event Timeline

I performed a search from the string 'ip' in any (WMF deployed) file containing the string 'tables'.

Search: https://codesearch.wmcloud.org/deployed/?q=ip&files=.*tables.*&excludeFiles=

I then read through the results looking for table columns containing 'ip' (ignoring core) and found possible hits in:

  • CheckUser
  • AbuseFilter
  • Echo
  • Flow
  • SecurePoll

...which we should look into.

Tchanders updated the task description. (Show Details)
ExtensionDoes it save IP addresses? What for?What should we do?
CheckUserYes. This is the one place where we'll be storing IP addresses, so this is fine.No action needed.
AbuseFilterThis stores IP addresses in abuse_filter_log.afl_ip if $wgAbuseFilterLogIP is true. This happens in WMF production: we currently store IP addresses in the AbuseFilter tables.Needs investigation.
FlowCommenting as a logged out user does not create a temporary account. Instead it saves the IP address in flow_revision.rev_user_ip and displays it in the UI. Does this even if IP actor creation in core errors out. More details: T345578#9290889.Flow needs to be disabled or updated. Growth-Team is working on this in T346108.

To do next

  • Echo
  • SecurePoll