In T27707#4922020, @brion wrote:

In T27707#4922017, @AlexisJazz wrote:

https://www.flickr.com/photos/tinto/30943950124/ and other photos from this Flickr user.

Perfect! Confirmed that the original file there fails to upload on test.wikipedia.org but uploads intact on my local instance with the patch. :D

I loaded the "naughty.png" on IE6 served as image/png. I see a fox. Perhaps your webserver was misconfigured in 2005?

I assure you I tested quite thoroughly at the time, and several people have independently discovered and reported the same vulnerability over the years. Here's another description, which includes an independently written sample PNG-containing-HTML exploit as well: https://forums.hak5.org/topic/6565-xss-exploit-in-ie-by-design-says-microsoft/

In any case, we can put this part on the back burner since keeping the exact reverse-engineered IE security checks doesn't seem to block JPEG files with exif data the same way the more expansive checks did... I'll do a more thorough investigation on removing them later on once I've got local XP & Vista instances to test with. Sounds like there may be more inconsistencies in terms of later changes in service packs that makes it hard to confirm in testing!

In T215128#4923631, @zhuyifei1999 wrote:

I upgraded youtube_dl on all hosts. Could you try again?

Anyhow, this task belongs to https://github.com/toolforge/video2commons/issues, not phabricator.

In T27707#4921905, @brion wrote:

The above patch https://gerrit.wikimedia.org/r/487527 removes some of the non-scripting tags from the checks in UploadBase::detectScript, and makes the conservative/exact IE heuristics called from UploadBase::verifyMimeType optional (but still on by default). This also deprecates $wgAllowTitleInSVG since <title is no longer looked for.

I think this should get JPEGs and such with links in their EXIF metadata working; are there some samples of files that were tripping up before that we can use to add in test cases? A test file I made locally with Gimp works but also already worked with the existing code because the EXIF data is after the area that got searched.

In T27707#4921804, @brion wrote:

In T27707#4920735, @AlexisJazz wrote:

Though this is not needed to remove this MIME sniffing check (or at the very least, make it optional), as I've shown it can't be exploited unless the webserver is misconfigured. So it doesn't apply to Wikimedia projects.

Can you clarify what you mean by "misconfigured"? No special configuration was required with exploit PNGs when I first tested this in 2005. ISTR that JPEG and GIF were not vulnerable when served with their native Content-Types because they have a different priority than PNG in the type checking.

The original 'naughty.png' sample from my 2005 testing is now available at https://brionv.com/misc/naughty.png -- note the way I embedded the HTML using a custom ancillary chunk might no longer be considered legit as some apps reject it now, but it could likely be retooled using a standard comment chunk. Breakdown of it is like this:

IE 8 on Windows 7 doesn't seem to be vulnerable and displays the image without running the script, but I still don't have an IE 6 or 7 test environment handy.

In T27707#4921089, @Aklapper wrote:

Logins should definitely be disabled for those.

This feels off-topic for this task. See https://wikitech.wikimedia.org/wiki/HTTPS/Browser_Recommendations#For_Users_of_Microsoft_Windows etc.

In T27707#4918648, @Tgr wrote:

In T27707#4915179, @brion wrote:

• This takes IE 6 off the table entirely.

IIRC we still get a bunch of IE6/7 visits from places which man-in-the-middle SSL connections.

Logins should definitely be disabled for those. That man (the one in the middle) can collect all their passwords. And to simplify things, those 4 users per million using IE7 on Vista should also have logins disabled. So logins can be disabled for all IE6 and IE7 users.

In T27707#4915140, @brion wrote:

The chance that someone would visit using an old IE version was probably 50% when the code was originally added; IE had a very high marketshare in the early 2000s. However at this point we can't even be accessed in IE 6 as far as I know (due to servers dropping old TLS versions for HTTPS). I think it's pretty fair to change the balance of what we check for.

In T27707#4918046, @Bawolff wrote:

In T27707#4918032, @AlexisJazz wrote:

In T27707#4918013, @Bawolff wrote:

As the file extension is restricted to a whitelist, the MIME type beyond the control of the attacker and invalid file signatures are also rejected, we must.. find a way around that I guess?

Usually in this scenario, its assumed file ext is .jpg, and the normal unix mime detection stuff would detect it as JPEG. But IE7 would in theory still treat as html (I think).

Actually no, the article Ghouston linked explains:

With the common GIF, JPEG and PNG formats, the browser ignores the result of MIME sniffing, as long as the filename extension, Content-Type and signature, all indicate the same type. Only if the results are inconsistent will Internet Explorer handle the file as the type identified by MIME sniffing.

So we really need to follow the Zany Scheme to exploit this. The scheme is a joke, but not inaccurate.

I think the microsoft docs say something different then Ghouston does:

If the server-provided MIME type is either known or ambiguous, the buffer is scanned in an attempt to verify or obtain a MIME type from the actual content. If a positive match is found (one of the hard-coded tests succeeded), this MIME type is immediately returned as the final determination, overriding the server-provided MIME type (this type of behavior is necessary to identify a .gif file being sent as text/html). During scanning, it is determined if the buffer is predominantly text or binary. Note In Microsoft Internet Explorer 6 for Windows XP Service Pack 2 (SP2), the MIME type "text/plain" is not ambiguous, and is never rendered as HTML in the restricted zone, even if the content suggests that this is the correct format.

From https://docs.microsoft.com/en-us/previous-versions/windows/internet-explorer/ie-developer/platform-apis/ms775147(v=vs.85)

Otherwise, this would basically be a non-issue even at the time [Additional cause for concern is I think IE6 can sometimes take extensions from query parameter part of the url]

In T27707#4918013, @Bawolff wrote:

As the file extension is restricted to a whitelist, the MIME type beyond the control of the attacker and invalid file signatures are also rejected, we must.. find a way around that I guess?

Usually in this scenario, its assumed file ext is .jpg, and the normal unix mime detection stuff would detect it as JPEG. But IE7 would in theory still treat as html (I think).

In T27707#4915179, @brion wrote:

Double-checking:

• IE 6 and 7 were both available on Windows XP, which includes TLS 1.0 support but not TLS 1.1 or 1.2. However IE 6-8 on XP fail to work anyway due to lack of SNI.
  • This takes IE 6 off the table entirely.
• IE 7 on Vista should still work correctly with TLS 1.0, 1.1, or 1.2 -- https://blogs.msdn.microsoft.com/kaushal/2011/10/02/support-for-ssltls-protocols-on-windows/
• IE8 supports X-Content-Type-Options: nosniff header; if we don't already use it consistently, applying this on all file views would resolve all sniffing issues on IE 8
  
  So the intersection of the danger zone, for Wikimedia sites, is:
• IE 7 running on Windows Vista, user logged in
  
  (This would be a bad experience for the user with no JS and probably no or broken styles, and it's going to be a very small percentage of people that will hit it ever.)
  
  For small internal sites, I assume you're just not going to have IE 6/7 so it shouldn't be a problem. ;)
  
  I think it's reasonably fair to just drop the anti-sniffing checks at this point, though we might want to consider explicitly disallowing logins on IE 7.

In T214853#4914769, @Zoranzoki21 wrote:

I tested, works as expected. And in browser and via command line. Can you still reproduce this?

zoranzoki21@zoranzoki21-eMachines-E525:~$ curl -I https://commons.wikimedia.org/wiki/Special:Log/GifTagger
HTTP/2 200 
date: Mon, 28 Jan 2019 16:25:15 GMT
content-type: text/html; charset=UTF-8
server: mw1325.eqiad.wmnet
x-content-type-options: nosniff
p3p: CP="This is not a P3P policy! See https://commons.wikimedia.org/wiki/Special:CentralAutoLogin/P3P for more info."
x-powered-by: HHVM/3.18.6-dev
content-language: en
expires: Thu, 01 Jan 1970 00:00:00 GMT
x-frame-options: DENY
backend-timing: D=284163 t=1548692715609284
vary: Accept-Encoding,Cookie,Authorization,X-Seven
content-encoding: gzip
x-varnish: 500400808, 245907591, 227728396
via: 1.1 varnish (Varnish/5.1), 1.1 varnish (Varnish/5.1), 1.1 varnish (Varnish/5.1)
accept-ranges: bytes
age: 0
x-cache: cp1075 pass, cp3032 pass, cp3040 pass
x-cache-status: pass
server-timing: cache;desc="pass"
strict-transport-security: max-age=106384710; includeSubDomains; preload
set-cookie: WMF-Last-Access=28-Jan-2019;Path=/;HttpOnly;secure;Expires=Fri, 01 Mar 2019 12:00:00 GMT
x-analytics: ns=-1;special=Log;https=1;nocookies=1
x-client-ip: 109.245.111.9
cache-control: private, s-maxage=0, max-age=0, must-revalidate
set-cookie: GeoIP=RS:00:Belgrade:44.82:20.47:v4; Path=/; secure; Domain=.wikimedia.org

In T27707#4910850, @Ghouston wrote:

Editing Exif without potentially breaking something is difficult. A lot of extensions have been added to JPEG over the years, and some of them contain pointers to data outside their own segments. Things like maker notes (proprietary and not fully documented) and Multi-Picture Format (which allows putting multiple images in a single JPEG file). They all need to be unpacked, adjusted, and repacked. The only practical method is probably to use ExifTool, and even that isn't perfect.

In T213885#4907782, @Jdforrester-WMF wrote:

In T213885#4907757, @AlexisJazz wrote:

@Jdforrester-WMF I don't really know this stuff, but could it be a problem that you are calling getMediaInfoViewRegex before you've defined it?

Sadly, no, that code file is loaded in one go before processing. The subsequent operations to name the slot clearly take place (otherwise it'd say <mw:mediainfoslotheader /> rather than "Structured data".

@Jdforrester-WMF I don't really know this stuff, but could it be a problem that you are calling getMediaInfoViewRegex before you've defined it?

In T214003#4897627, @Tgr wrote:

@AlexisJazz If you mean at the exact same time, it's extra work and does not seem necessary. If you mean within a few days of each other, that's doable, sure. I was waiting for a reply on my comment.

As zhuyifei1999 says above, only the order matters. If the group change happens before the UW change, people would see the "Share images from Flickr" button but will be stopped by the abuse filter. Those who decide to ignore the abuse filter (just hit "upload" again) would need their uploads to be manually fixed. Which is doable for a few days, probably, as I've dealt with it before for extended uploaders, but messy and no smooth user experience. Merge the UW change first and everything will be fine.

@Multichill where should I propose a change? It should be changed globally, so I guess somewhere on meta?

It would be ideal if https://gerrit.wikimedia.org/r/#/c/operations/mediawiki-config/+/485487/ (Merge the "extended-uploader" and "autopatrolled" user groups on Commons) and https://gerrit.wikimedia.org/r/#/c/mediawiki/extensions/UploadWizard/+/485141/ (mw.FlickrChecker: Use {{flickrreview}}) could be merged at the same time. (or merge the UploadWizard patch first, that'll be fine too)

In T213571#4893376, @Jdforrester-WMF wrote:

In T213571#4892511, @AlexisJazz wrote:

In T213571#4879390, @Jdforrester-WMF wrote:

In T213571#4875391, @AlexisJazz wrote:

@Jdforrester-WMF According to meta, level 0 doesn't understand the language at all. Level 1 can only read. Only level 2 and up are capable or writing in a language. So input fields (which require writing) should only be presented to those who consider themselves level 2 and up.

I appreciate that that random page on Meta that I'd never seen before says that, and made you think that adding dozens of -0 as a talisman to scare off users who speak those languages was a supported way to use the Babel extension, but it isn't and it wasn't.

I would very strongly recommend that you delete the mess of -0 boxes on your user page that mostly highlight your account to people that speak those languages (and disrupt your experience on Meta, with language links on the sides of articles on Wikipedia etc., on Wikidata, and now on Commons).

I would very strongly recommend to make language configuration a preference, which it should have been in the first place.

I agree that it is both surprising and confusing that the Babel extension combines fluff (user boxes) with configuration and does so via user pages. However, that ship sailed eleven years ago in 2008 when it got created, sadly, and changing it now would be very disruptive for users. I'd be in favour of initiating some work to re-evaluate this, probably eventually via a Tech RfC after a wide-ranging investigation into possible uses and improvements, but that's far outwith the scope of the SDC project, and would have to be done in communion with the Language team (as the experts), the Wikidata team (as one of the affected parties), many cross-language editors, and no-doubt other stakeholders.

Also, please don't edit-war over task titles. Making explicit that a task is asking for a deviation in behaviour between similar features is important to set context.

In T213571#4879390, @Jdforrester-WMF wrote:

In T213571#4875391, @AlexisJazz wrote:

@Jdforrester-WMF According to meta, level 0 doesn't understand the language at all. Level 1 can only read. Only level 2 and up are capable or writing in a language. So input fields (which require writing) should only be presented to those who consider themselves level 2 and up.

I appreciate that that random page on Meta that I'd never seen before says that, and made you think that adding dozens of -0 as a talisman to scare off users who speak those languages was a supported way to use the Babel extension, but it isn't and it wasn't.

I would very strongly recommend that you delete the mess of -0 boxes on your user page that mostly highlight your account to people that speak those languages (and disrupt your experience on Meta, with language links on the sides of articles on Wikipedia etc., on Wikidata, and now on Commons).

In T214003#4890283, @Majora wrote:

@AlexisJazz Obviously there was some confusion between user groups and user rights which is understandable. I personally don't have a problem with including the patroller user group in this request since, as you said, patroller is an "upgrade" to the autopatrolled group as it is currently defined on Commons. I didn't read your original proposal as meaning "add upload_by_url to every group that has the autopatrol flag" but I guess I could read it that way also. If another RfC is required to grant the flag to the patroller group as well then so be it. But that would be up to the devs who actually make those changes whether or not they want to include the patroller group since it is set up to be an extension of autopatrolled on Commons.

In T89131#4888586, @Tgr wrote:

In T89131#4888045, @AlexisJazz wrote:

https://commons.wikimedia.org/wiki/Commons:Village_pump/Proposals#Give_autopatrolled_users_more_upload_options was passed.

I hope whoever wrote "If we accept this, T89131 won't take long." in that proposal really meant T100062, because right now it's unclear if anyone is planning to work on this task.

@Zoranzoki21 as you claimed this task, do you agree the proposal included all autopatrolled users, so also patrollers and bots? If not, I'll just request for those 687 users to be granted autopatrol.

In T214003#4888702, @Majora wrote:

In T214003#4888247, @AlexisJazz wrote:

In T214003#4888199, @DonTrung wrote:

Would this then also automatically give these permissions to "patrollers"?

Autopatrol is included in patrol.

That's actually not how that works @AlexisJazz. You have to separate user groups from user rights/flags. Flags are assigned to groups and then the group is granted to editors. Right now both the autopatrolled and patroller groups have the "autopatrol" flag. The discussion and consensus at the pump was to grant the "upload_by_url" flag to the autopatroller group (which would have made the extended uploader group a duplicate). There was no talk of, and no consensus, to add this flag to the patroller group. So to answer @DonTrung, no. The flag will not be granted to patrollers.

@Aklapper I suspect this change would not be desired outside of Wikimedia projects, perhaps even outside of Wikimedia Commons. I don't know if/how this can be applied only to Commons. Maybe changing the FlickreviewR bot is better, but I'll let zhuyifei1999 comment on that.

In T214003#4888199, @DonTrung wrote:

Would this then also automatically give these permissions to "patrollers"?

@zhuyifei1999 just in case anyone is going to make a problem out of the above, can you alter FlickreviewR to start chomping on {{FlickrVerifiedByUploadWizard.*}} tags?

Here are the changes needed (really, do you need me for this?):

Also see T214003.

Also see T89131, please replace or supplement {{FlickrVerifiedByUploadWizard.*}} with {{flickrreview}}.

@Aklapper I'm not aware of your conventions.

In T89131#3859557, @Steinsplitter wrote:

Since it is causing issues (e. g. here) we should probably replace "{{FlickrVerifiedByUploadWizard.*}}" with "{{flickrreview}}" in the code. OR we simply add {{flickrreview}} for a additionally check (then we can remove the FlickrVerifiedByUploadWizard from AbuseFilter). I actually have no idea for a onwiki-only solution here. But both changes are trivial.

https://commons.wikimedia.org/wiki/Commons:Village_pump/Proposals#Give_autopatrolled_users_more_upload_options was passed.

@Multichill You didn't answer my question. If you add en-0 to your babel it reads "This user has no knowledge of English (or understands it with considerable difficulty)". Several languages even bold the "no knowledge" part. The user has no knowledge. Why should those users be presented with input fields for languages they don't know?

@Multichill : I'm puzzled.

In T213571#4873929, @Lydia_Pintscher wrote:

It was not forgotten. It is an intentional feature on Wikidata.

In T213571#4873915, @Lydia_Pintscher wrote:

It should stay like it is for standard Wikibase at least. People use it.

According to @matmarex this is an issue in IE5-7. MS dropped support for IE7 entirely in April 2016. IE8 was released in 2009. According to https://en.wikipedia.org/wiki/Internet_Explorer_7, as of May 2012, estimates of IE7's global market share were 1.5-5%. According to netmarketshare.com the market share is now 0.16%.

@Ammarpad I still don't see it? The F template is used to link https://commons.wikimedia.org/wiki/File:Full-protection-shackle.svg, not https://commons.wikimedia.org/wiki/File:Full-protection-shackle-double.svg?

@Aklapper @Shreyasminocha can't you just remove line 1332 ("'<a href',") from UploadBase.php? Is that Internet Explorer bug still a thing? Otherwise can you check if it's part of the EXIF and ignore it if that's the case?

In T212693#4845910, @Aklapper wrote:

@AlexisJazz: Please include clear steps to reproduce. Which of the many available ways to upload files did you use exactly?

Edit: Ah, I assume the "MediaWiki/UploadWizard" in the last line implies that.

@Aklapper okay, sorry, I didn't know how to tag this as a feature request. Can the mobile app convert it? (I guess that would only help for Android, which is maybe more likely to use .webp anyway..) Can it be converted with JavaScript? Could a project an Wikimedia Toolforge handle it?

"It should still be fine to add FlickrVerifiedByUploadWizard if the account is trusted, however (for example administrators)."

@Slowking4 is not actually in the extended uploaders group. I suspect he has the "Share images from Flickr" button because he is in the "GWToolset users" group.

@kaldari no problem, here it is, with free patch!! T210339

In T210859#4791099, @Aklapper wrote:

@AlexisJazz: I'm looking at https://nl.wikipedia.org/w/index.php?title=Overleg:Theo_van_Gogh_(kunsthandelaar)&oldid=52708637 but which exact "image on the talk page" do you refer to? Not sure how to see the problem... :-(

In T124101#4789680, @Tgr wrote:

In T124101#4789594, @AlexisJazz wrote:

But the things that take me hours could be done in seconds with shell access.

How do you think that would happen? If you have specific steps in mind, maybe we could expose those to power users. I can't think of any investigation that would be much more complicated without shell access though.

@Incnis_Mrsi at T198177 I also found some missing revisions. But the things that take me hours could be done in seconds with shell access.

In T198177#4433629, @Jdforrester-WMF wrote:

I'm de-prioritising this as it's unrecoverable and the general comments are valid but not new.

@zhuyifei1999 "Thanks. How much bitrate do you think we should use? I'm thinking of 128k"

@zhuyifei1999 "The problem: opus encoder doesn't seem to allow constant quality,"

@TheDJ I plan to convert some more after T209557 is fixed..

Just purged again (5 days after my Wikidata edit), still "in use".

@brion https://meta.wikimedia.org/wiki/Have_the_patents_for_MPEG-4_Visual_expired_yet%3F

I don't know, you tell me?

I think I'm gonna have a serious problem with this.

I had opened T207883, unaware of this request.