Missing return-value validation of the function PyArray_DescrNew

[awen-li]

Reproducing code example:

The definition of PyArray_DescrNew

NPY_NO_EXPORT PyArray_Descr * PyArray_DescrNew(PyArray_Descr *base)
{
    PyArray_Descr *newdescr = PyObject_New(PyArray_Descr, Py_TYPE(base));
    if (newdescr == NULL) {
        return NULL; ----------------> **point 1**
    }
    .........
    if (base->c_metadata != NULL) {
        newdescr->c_metadata = NPY_AUXDATA_CLONE(base->c_metadata);
        if (newdescr->c_metadata == NULL) {
            PyErr_NoMemory();
            /* TODO: This seems wrong, as the old fields get decref'd? */
            Py_DECREF(newdescr);
            return NULL; ----------------> **point 2**
        }
    }
    ........
    if (newdescr->subarray) {
        newdescr->subarray = PyArray_malloc(sizeof(PyArray_ArrayDescr));
        if (newdescr->subarray == NULL) {
            Py_DECREF(newdescr);
            return (PyArray_Descr *)PyErr_NoMemory();   ----------------> **point 3**
        }
    }
   .........
    return newdescr;
}

Call-site example for PyArray_DescrNew

NPY_NO_EXPORT PyArray_Descr *
PyArray_DescrNewByteorder(PyArray_Descr *self, char newendian)
{
    PyArray_Descr *new;
    char endian;

    new = PyArray_DescrNew(self);
    endian = new->byteorder; -----> direct read through "new“ 
    ......
}

Error message:

At most call-sites for PyArray_DescrNew, there are no validations of its return,
but an invalid address may be returned.
example

NumPy/Python version information:

the main branch

[awen-li]

Anyone can help confirm this issue? thanks.

[eric-wieser]

This looks valid to me, but probably only happens if memory is exhausted. A patch would be appreciated.

[00xc]

Hi, any update on this issue? It was recently assigned CVE-2021-41495.

[mattip]

@00xc do you know how those CVEs are being generated? This is the second one in two days, and both of them seem rather far fetched. I would hate for NumPy to have to enter a sudden race to close issues in a mode of "CVE-driven development".

[00xc]

@00xc do you know how those CVEs are being generated? This is the second one in two days, and both of them seem rather far fetched. I would hate for NumPy to have to enter a sudden race to close issues in a mode of "CVE-driven development".

I have no idea who is requesting them - we get CVE reports from our automated fetchers. I do agree that this and CVE-2021-41496 (which I assume is the other CVE you're referencing) seem to be far fetched, and not too security critical, but I just thought pinging upstream would not hurt.

[mattip]

Thanks for the heads up.

[legendlc]

Hi, any progress on this? Thanks.
The CVSS score of this vulnerability seems to outweigh its impact 🤔

[seberg]

This is only likely (or even possible) to cause issues when you are already out of memory, in that case you can a process crash, rather than a Python interpreter shutdown, I guess? ;)

These CVE reports (with such ridiculous scores for what I understand), are a serious nuisance by now. But I am not sure what we should do about it easily.

[attritionorg]

As a general FYI on the CVE and other vulnerability databases (VDB) process, one requirement of CVSS is "score for the worst". If a vulnerability is confirmed by the vendor (this was) and the impact is not entirely understood (like this), it will be scored for what the worst outcome might be. In this case, a full remote DoS which gives the CVSSv3 7.5 score.

As for what you can/should o? If there are limiting factors that make this harder to exploit (increasing access complexity) or the impact is not severe (partial availability), the vendor or researcher briefly explaining that is a huge help to outside parties that don't work with the software every day. Then the VDB analysts can more easily understand the issue and score accordingly.

[seberg]

But, a user cannot trigger this except by first exhausting memory (which was pointed out in the confirmation). Further, if someone can run arbitrary NumPy code they can already trivially DoS you (or worse), so the main security consideration that I see for NumPy at this point is malicious data. It is unclear to me that this fits the bill (or any/most of the other CVEs here).

I see multiple issues:

• This probably creates a lot of unnecessary work for downstream to review users who use NumPy well isolated from any malicious user.
• We were not asked for confirmation beyond on the issues here (which all seemed low priority for good reasons). We could have explained why they seemed not very important nobody bothered too much about it.
• It is not a great dynamic if we end up having to scramble to fix meaningless CVEs.

[attritionorg]

"but probably only happens if memory is exhausted." That doesn't specify who can exhaust memory. It implies the attacker might be able to do that so "assume the worst" kicks in.

Your caveat of "if someone can run arbitrary NumPy code..", is great to point out. Those are the kinds of clarifications that are immensely helpful to third-party analysts not intimately familiar with the software.

re: "if this fits the bill". The general rule for a 'vulnerability' is if privilege boundaries can be crossed. Can a user introduce malicious code that lets them do something they should not be able to? In this case, create a memory exhaustion (limited DoS) scenario? Something else? Or is this all entirely moot because a user is expected to be trusted at the point they introduce that code? If that is the case, a vendor giving a clear dispute is what CVE needs. Once the ID is assigned, you can't un-ring that bell so to speak. But MITRE can flag an entry as "Vendor Disputed" and give the caveat, often quoted verbatim from the vendor. So a clear/concise one-liner explaining why it isn't a vulnerability is the best you can do if an erroneous report is filed (or the report is accurate, but not qualified as to the conditions for triggering).

The CVE ecosystem these days is not designed to get vendor confirmation. The original purpose of CVE was to assign a candidate ID (e.g. CAN-2022-0001) while it was discussed and determined to be valid (which happened behind the scenes of closed-source vendors typically). If not, it would be rejected and that CVE ID would never see the light of day. If valid, it would move to full ID and change from CAN to CVE. The software ecosystem changed, MITRE changed the CVE process, and the 'CAN' process was removed because it was effectively obsolete by ~ 2006.

These days just the presence of a CVE can be incredibly troublesome for vendors for many reasons, and add a lot of work that they should never have had to deal with. They almost invariably begin with an incomplete or inaccurate bug report that makes it seem worse than it is. I feel really bad for vendors that have to deal with this, but that is how CVE works unfortunately.

[rgommers]

Your caveat of "if someone can run arbitrary NumPy code..", is great to point out. Those are the kinds of clarifications that are immensely helpful to third-party analysts not intimately familiar with the software.
...
Or is this all entirely moot because a user is expected to be trusted at the point they introduce that code?

This is true here, and pretty much always true for CVEs we get, that's what is so frustrating. It's a Python library for numerical analysis, that for 99.9x% of usage is not exposed to untrusted users. The only users that should really worry about this are things like services that provide hosted runners (Binder, Colab, etc.). And those allow executing arbitrary Python code, and hence they are very much aware that they should sandbox and secure their runners.

The general rule for a 'vulnerability' is if privilege boundaries can be crossed.

You could argue that there's things like HPC systems which are shared between users, and those could use a vulnerability to cross a boundary. Which is technically true, but those users are typically already trusted (e.g. employees or students within an organization), which is hugely different from CVEs related to, say, components from which websites are built. So yes, it's indeed largely moot.

.... I feel really bad for vendors that have to deal with this, but that is how CVE works unfortunately.

Thanks for the detailed explanation @attritionorg!

[attritionorg]

MITRE/CVE less so, but other databases look at libraries a bit different. We have to consider "what is the base vuln" vs "how can this be implemented". In some cases we have no idea how a typical integration works which speaks to your comment "99.9x% of usage is not exposed to untrusted users". This is extremely helpful for us since it tell us the likelihood of an attack vector are slim. We'd be more inclined to ignore the report completely unless A) vendor or researcher gives a real-world scenario and we know to be cautious when a researcher provides one B) a CVE is assigned. Then we have to cover it but, we provide value in disclaiming it. In VulnDB, we actually flag entries as "Not a Vuln" in many of these cases and provide a technical note explaining why. That way our users know to ignore the report and move on. Meanwhile, vendors are left to fight with MITRE/CVE to get it disputed which can be an uphill battle.

So again, I really stress that even if it is brief, giving a clear indication of the severity and exploit path, if any, helps us immensely. Thanks!

[seberg]

Thanks! We need to discuss if its worthwhile or not to dispute, I think. Right now I am just a bit curious if something like this would be a good "dispute" (that we could probably reuse verbatim for de-facto all of these):

Not a meaningful vulnerability because triggering the issue seems only plausible if the malicious party already has the privilege to run NumPy commands. Thus, while a bug, it does not present an escalation of privilege.

[rgommers]

We need to discuss if its worthwhile or not to dispute, I think. Right now I am just a bit curious if something like this would be a good "dispute" (that we could probably reuse verbatim for de-facto all of these):

We probably should, but at the same time let's also get it fixed and backported to 1.22.1, because it does affect some people and we're going to continue to get noise on this issue.

[theintz]

Safety is reporting all versions of numpy unsafe as of this morning. It was presumably added with the february update. This is what it reports now:

docker run [image] pipenv check --system
Checking PEP 508 requirements...
Passed!
Checking installed package safety...
44715: numpy >0 resolved (1.22.1 installed)!
All versions of Numpy are affected by CVE-2021-41495: A null Pointer Dereference vulnerability exists in numpy.sort, in the PyArray_DescrNew function due to missing return-value validation, which allows attackers to conduct DoS attacks by repetitively creating sort arrays.
https://github.com/numpy/numpy/issues/19038

Our build pipelines are currently broken due to this, we will bypass safety for the time being. Thanks everyone working to fix this!

[krukas]

Our build pipelines are currently broken due to this, we will bypass safety for the time being. Thanks everyone working to fix this!

You can add --ignore=44715 to safety command so you can still run it, but ignore the CVE-2021-41495 check

[GMaxera]

We need to discuss if its worthwhile or not to dispute, I think. Right now I am just a bit curious if something like this would be a good "dispute" (that we could probably reuse verbatim for de-facto all of these):

We probably should, but at the same time let's also get it fixed and backported to 1.22.1, because it does affect some people and we're going to continue to get noise on this issue.

Any update? ... still making noise ;-) because the 1.22.1 is reported to be vulnerable by this CVE.
So, will be fixed? and if so, on which release?

[rizerzero]

I see 1.22.2 is out, but the CVE has not been addressed. Our safety check still fails:

You can ignore it with --ignore 44715

[rgommers]

@charris I see that this didn't make it into 1.22.3 - I added the 1.22.4 milestone, closed 1.22.3, and also moved the other remaining 6 issues on that milestone to 1.22.4

[seberg]

This was already part of 1.22.2, we just may need to ping the CVE database folks again to update their entry.

[rgommers]

This was already part of 1.22.2, we just may need to ping the CVE database folks again to update their entry.

Yes indeed, thanks - some of the comments above were unclear and it wasn't marked. It's indeed in 1.22.2, the backport PR was gh-20984. I have added the milestone now.

One comment above says "Take into account that the safety check database is updated only once a month. So, you need to wait next month to see something changed there". That was a month ago, so I hope it's up-to-date now.