bmlt-enabled / bread

BMLT Meeting List Generator Plugin For Wordpress https://wordpress.org/plugins/bread/
https://wordpress.org/plugins/bread/
GNU General Public License v2.0
6 stars 9 forks source link

Remove use of curl disabling TLS verification #58

Closed dgershman closed 5 years ago

dgershman commented 5 years ago

Wordpress wants us to rip out curl references in any of the bread code.

dgershman commented 5 years ago

this is going to require making some custom changes to mpdf.

dgershman commented 5 years ago

looks like we are not using this feature

https://github.com/mpdf/mpdf/blob/b1e0f683529dbeceebc1d032c2e207e92b85eb67/src/RemoteContentFetcher.php#L44

dgershman commented 5 years ago

They have just confirmed that we are ok. Attaching the email thread here for historical purposes.

Cool then you're fine :)

> I just verified and the plugin is not making use the of this feature which
> requires setting the configuration
> 
> https://github.com/mpdf/mpdf/blob/b1e0f683529dbeceebc1d032c2e207e92b85eb67/src/RemoteContentFetcher.php#L44
> <https://github.com/mpdf/mpdf/blob/b1e0f683529dbeceebc1d032c2e207e92b85eb67/src/RemoteContentFetcher.php#L44>
> 
> This plugin is not making use of this feature.
> 
> —Danny
> 
> > On Jan 10, 2019, at 2:18 PM, Ipstenu (Mika Epstein)
> <plugins@wordpress.org> wrote:
> > 
> > 
> > In this case? DON'T disable TLS verification. 
> > 
> > The whole reason you got flagged is that we scanned and your library,
> which is where you use curl, DOES.
> > 
> >> Can you define acceptable SAFE usage of curl?
> >> 
> >> On Thu, Jan 10, 2019 at 10:29 AM Ipstenu (Mika Epstein) <
> >> plugins@wordpress.org> wrote:
> >> 
> >>> 
> >>> From a security standpoint yes.
> >>> 
> >>> From a usability one, no. Many web hosts block file_get_contents from
> >>> remote access.
> >>> 
> >>>> Is file_get_contents considered safe in your minds?
> >>>> 
> >>>>> On Jan 10, 2019, at 9:33 AM, Ipstenu (Mika Epstein)
> >>>> <plugins@wordpress.org> wrote:
> >>>>> 
> >>>>> 
> >>>>> I’m telling you that a depenancy you use is UNSAFE and may end up
> >>>> requiring us to remove your plugin, should that be exploited. You
> >> should
> >>>> talk to the vendor of the library directly, or like i said, make sure
> >>> YOUR
> >>>> USE is safe.
> >>>>> 
> >>>>> I know it sucks, but part the job as a plugin developer is package
> >>>> management.
> >>>>> 
> >>>>>> I do not author http://mpdf.github.io <http://mpdf.github.io/>, you
> >>> are
> >>>>>> basically telling me to rip out a dependency that powers the bulk
> >> my
> >>>>>> plugin.
> >>>>>> 
> >>>>>>> On Jan 10, 2019, at 9:21 AM, Ipstenu (Mika Epstein)
> >>>>>> <plugins@wordpress.org> wrote:
> >>>>>>> 
> >>>>>>> 
> >>>>>>> Then that’s why. It’s using curl in an insecure way, and you
> >> need
> >>>> to
> >>>>>> either not use it OR ensure your use is, indeed, safe.
> >>>>>>> 
> >>>>>>>> Yes I couldn’t find any.  Only in the mpdf dependency, which I
> >>>> really
> >>>>>>>> have
> >>>>>>>> no control over.
> >>>>>>>> 
> >>>>>>>> On Thu, Jan 10, 2019 at 9:14 AM Ipstenu (Mika Epstein) <
> >>>>>>>> plugins@wordpress.org> wrote:
> >>>>>>>> 
> >>>>>>>>> 
> >>>>>>>>> You mean you don’t know what files call curl directly in your
> >> own
> >>>>>>>> code?
> >>>>>>>>> 
> >>>>>>>>>> Can you provide more details about which places you are seeing
> >>> this
> >>>>>>>>> occur?
> >>>>>>>>>> 
> >>>>>>>>>>> On Jan 8, 2019, at 5:43 PM, Plugin Directory at WordPress.org
> >>>>>>>>>> <plugins@wordpress.org> wrote:
> >>>>>>>>>>> 
> >>>>>>>>>>> Hello radius314,
> >>>>>>>>>>> 
> >>>>>>>>>>> On a regular review of plugins hosted on WordPress.org, we
> >> have
> >>>>>>>>>> determined a number of plugins are disabling TLS verification.
> >>> This
> >>>>>>>>>> generally happens when a developer includes direct CURL calls
> >> in
> >>>>>> their
> >>>>>>>>>> code which can be harmful for end users in certain situations.
> >>>>>>>>>>> 
> >>>>>>>>>>> This impacts the following plugins you have commit access to:
> >>>>>>>>>>> 
> >>>>>>>>>>> * https://wordpress.org/plugins/bread
> >>>>>>>>>>> 
> >>>>>>>>>>> We recommend you cease including your own CURL code and
> >> instead
> >>>> use
> >>>>>>>> the
> >>>>>>>>>> HTTP API. It's both faster and more extensive. It'll fall back
> >> to
> >>>>>> CURL
> >>>>>>>> if
> >>>>>>>>>> it has to, but it will use a lot of native WordPress
> >> functionality
> >>>>>>>> first.
> >>>>>>>>>> Most importantly, it won't disable TLS verification. Learn more
> >>>> about
> >>>>>>>> the
> >>>>>>>>>> HTTP API here:
> >> https://developer.wordpress.org/plugins/http-api/
> >>>>>>>>>>> 
> >>>>>>>>>>> For an in-depth analysis of the issue, its impact, and why we
> >>>>>>>>> discourage
> >>>>>>>>>> the use of CURL, please review the following links:
> >>>>>>>>>>> 
> >>>>>>>>>>> *
> >>>>>>>>>> 
> >>>>>>>>> 
> >>>>>>>> 
> >>>>>> 
> >>>> 
> >>> 
> >>
> https://paragonie.com/blog/2017/10/certainty-automated-cacert-pem-management-for-php-software
> >>>>>>>>>>> *
> >>>>>>>>>> 
> >>>>>>>>> 
> >>>>>>>> 
> >>>>>> 
> >>>> 
> >>> 
> >>
> https://www.htmlcenter.com/blog/please-stop-using-curl-in-wordpress-plugins/
> >>>>>>>>>>> 
> >>>>>>>>>>> Keep in mind, WordPress includes cacert.pem so if you use the
> >>> HTTP
> >>>>>>>> API
> >>>>>>>>>> you won't need to.
> >>>>>>>>>>> 
> >>>>>>>>>>> Please let us know if you have any questions. If you no longer
> >>>> wish
> >>>>>>>> to
> >>>>>>>>>> maintain your plugins, simple reply to let us know and we can
> >>>>>>>> permanently
> >>>>>>>>>> close them for you.
> >>>>>>>>>>> 
> >>>>>>>>>>> - WordPress Plugin Directory team
> >>>>>>>>>>> 
> >>>>>>>>>>> 
> >>>>>>>>> 
> >>>>>>>>> 
> >>>>>>>>> 
> >>>>>>>>> --
> >>>>>>>>> Ipstenu (Mika Epstein)
> >>>>>>>>> 
> >>>>>>>>> 
> >>>>>>>>> 
> >>>>>>> 
> >>>>>>> 
> >>>>>>> 
> >>>>>>> 
> >>>>> 
> >>>>> 
> >>>>> 
> >>>>> 
> >>> 
> >>> 
> >>> 
> >>> --
> >>> Ipstenu (Mika Epstein)
> >>> 
> >>> 
> >>> 
> > 
> > 
> > 
> >

-- 
Ipstenu (Mika Epstein)
pjaudiomv commented 5 years ago

awesome, I have had a similar conversation with that exact guy when trying to get the bmlt-versions plugin accepted. it was unpleasant.