Skip to content

MAINT: compensate for NumPy load change#2257

Merged
richardjgowers merged 1 commit intoMDAnalysis:developfrom
tylerjereddy:np_load_cve_compensate
Apr 29, 2019
Merged

MAINT: compensate for NumPy load change#2257
richardjgowers merged 1 commit intoMDAnalysis:developfrom
tylerjereddy:np_load_cve_compensate

Conversation

@tylerjereddy
Copy link
Member

Fixes recent CI failures -- see for example #1991.

Related to: numpy/numpy#12889
And CVE: https://nvd.nist.gov/vuln/detail/CVE-2019-6446

Basically, someone decided it would be a good idea to open a government / software vulnerability ticket because np.load() had allow_pickle=True as the default, which theoretically might be exploited to execute arbitrary code.

MDAnalysis has a usage of np.load() in the encore machinery that was depending on the original default, which has now been switched to comply with the CVE (otherwise, various corporations / government orgs can't use latest NumPy with an outsanding CVE).

My fix here is the simplest one possible. Of course, one might argue that someone could just open a CVE about MDAnalysis now, but seriously maybe we can just patch to restore the original behavior and move on?

If we really want to be more secure, I suspect we may need a new kwarg somewhere in the encore machinery, which we might use in our testing, but not make the default to users.

So, maybe using loadz(self, fname, allow_pickle=False): as a new signature, with True used in our testing. I suppose that may be a little more secure, but haven't tested this locally yet to see the implications.

* upstream changes to np.load now require
us to specify allow_pickle=True in encore
machinery
@codecov
Copy link

codecov bot commented Apr 29, 2019

Codecov Report

Merging #2257 into develop will increase coverage by 0.35%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #2257      +/-   ##
===========================================
+ Coverage     89.3%   89.66%   +0.35%     
===========================================
  Files          156      172      +16     
  Lines        19407    21396    +1989     
  Branches      2783     2783              
===========================================
+ Hits         17332    19184    +1852     
- Misses        1473     1616     +143     
+ Partials       602      596       -6
Impacted Files Coverage Δ
package/MDAnalysis/analysis/encore/utils.py 83.45% <100%> (ø) ⬆️
...age/MDAnalysis/analysis/hbonds/hbond_autocorrel.py 87.8% <0%> (-0.54%) ⬇️
core/util.py 100% <0%> (ø)
transformations/__init__.py 100% <0%> (ø)
auxiliary/base.py 89% <0%> (ø)
util.py 88.15% <0%> (ø)
topology/base.py 97.67% <0%> (ø)
__init__.py 91.89% <0%> (ø)
topology/__init__.py 100% <0%> (ø)
visualization/__init__.py 100% <0%> (ø)
... and 21 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c7c394e...f11555f. Read the comment docs.

@tylerjereddy
Copy link
Member Author

Lots of strange issues on Travis

@richardjgowers richardjgowers merged commit 2196bbd into MDAnalysis:develop Apr 29, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants