Currently we use some variables in AbuseFilterVariableHolder to store internal info about the context: in fact, we have a 'context' variable inside mVars which we sometimes write/read to (see grep). That variable is also included in $varBlacklist, which prevents the export of listed items. If a user sets a 'context' variable inside filters, it actually seems to work, however it won't be dumped in logs as T170504 will be implemented.
Since (at least to me) it's not totally clear how such variables work, and since for the mentioned task I'd also need a reserved one, my proposal is to clearly reserve some variables, i.e. throw an exception if the user tries to overwrite them. However, we first need a query to see if a variable named 'context' is ever defined somewhere. It shouldn't be difficult, we only need a query like SELECT af_id FROM abuse_filter WHERE af_pattern LIKE '%context :=%' . We may also rename 'context' to something more difficult to overwrite, e.g. 'af_system_context', but I'm afraid old filters may react badly like it happened in T191696.
Description
Details
| Subject | Repo | Branch | Lines +/- | |
|---|---|---|---|---|
| Prevent the user from overriding blacklisted variables | mediawiki/extensions/AbuseFilter | master | +5 -2 |
Related Objects
- Mentioned In
- T193894: Run listed queries for all wikis in order to unblock existing changes
T191978: Devise a process for finding and fixing filters that will be affected by changes in AbuseFilter - Mentioned Here
- T170504: Show the value of custom variables in the details view of AbuseFilter
T191696: Abuse filter error (Exception caught: Unknown variable compute type subtract)
Event Timeline
Change 424850 had a related patch set uploaded (by Daimona Eaytoy; owner: Daimona Eaytoy):
[mediawiki/extensions/AbuseFilter@master] Prevent the user from overriding blacklisted variables
We need a subtask for the search and modification of the existing filters with a "context" variable in WMF.
@Huji Do we need a proper subtask or is it fine to do everything here? Actually, we'd need queries for several open tasks, is there an 'official' way to ask for them or should we just wait?
That is what I asked of @Bawolff in one of the other tasks. He responded to my question partially and copied @APalmer_WMF from legal too. I think this whole discussion (of what is the proper way to ask for queries and to fix filters enmass) deserves a task of its own! I will copy all of you in a task I am about to create in a moment!
@Huji Yes please! It doesn't make much sense to just let requests sit until someone will notice them by chance.
The same problem goes for global_log_ids and local_log_ids. These variables are stored and used in a way that it doesn't conflict with real variables, but what if the code is changed? A slight change may made those variables overwrite user-set ones, so we'd better avoid any risk. Also, we're using an "internal storage" for "meta-data", which isn't that good practice.
Change 424850 merged by jenkins-bot:
[mediawiki/extensions/AbuseFilter@master] Prevent the user from overriding blacklisted variables