Repository navigation
Insecure string comparison (incomplete comparison) in _convert_from_str of descriptor.c #18993
Description
Activity
I doubt it matters (its not like this is relevant for security or anything). But adding the
+1would be good, also to avoid the slightly wrong things being copied.- changed the title
[-]Unsecure string comparison (incomplete comparison) in _convert_from_str of descriptor.c[/-][+]Insecure string comparison (incomplete comparison) in _convert_from_str of descriptor.c[/+]on May 13, 2021 Do you mind telling me the tool that was used to reproduce to get that error message?
- Forked numpy and then cloned it
- Ran python tests using "python3 runtests.py -v" and the build passed
- The change I made was the following
diff --git a/numpy/core/src/multiarray/descriptor.c b/numpy/core/src/multiarray/descriptor.c index 58aa608c3..f6202c7ea 100644 --- a/numpy/core/src/multiarray/descriptor.c +++ b/numpy/core/src/multiarray/descriptor.c @@ -1729,7 +1729,7 @@ _convert_from_str(PyObject *obj, int align) for (int i = 0; i < ndep_tps; ++i) { char *dep_tp = dep_tps[i]; - if (strncmp(type, dep_tp, strlen(dep_tp)) == 0) { + if (strncmp(type, dep_tp, strlen(dep_tp)) + 1) { /* Deprecated 2020-06-09, NumPy 1.20 */ if (DEPRECATE("Numeric-style type codes are " "deprecated and will result in "
- Ran the tests again and received the following error: ERROR numpy/core/tests/test_regression.py - TypeError: data type 'int32' not understood
Do you know where I might of gone wrong and why I am seeing this error?
That change is nonsense, you removed the
==.Reacted by NectDz and dedronekYou removed the
== 0, that would invert the logic.Reacted by NectDz- added a commit that references this issue
on Aug 13, 2021 I realize the issue is closed, but would like to note that it has been assigned CVE-2021-34141
Reacted by NectDz, Jose de Jesus Medina, Alan Verdugo and Neil- This code doesn't exist anymore anyway, but can someone explain what even the point of a CVE is here? At this point, the CVE feels just like useless alarming about completely harmless things.
27 remaining items
Hi，can someone helps? Is it properly to update "strlen(dep_tp)" to "strlen(dep_tp)+1" in the line "if (strncmp(type, dep_tp, strlen(dep_tp)) == 0) {" within numpy-1.16.5 to fix cve-2021-34141? We do not want to upgrade for some reasons. Thanks.
@seberg @rgommers@Ren0216 I'd recommend re-applying the fix from gh-19539, and possible previous fixes to the same code. Or just leave it alone, since @seberg explained in the comment above that this CVE isn't valid. Trying to come up with a new fix isn't a good idea, it's much more likely to cause problems than solve them.
- added a commit that references this issue
on Apr 19, 2022 From what I can see the original code was actually correct: it's checking whether
dep_tpis a prefix oftype, so that (for example)Uint64will be flagged as deprecated because it hasUintas a prefix:>>> np.dtype("Uint64") __main__:1: DeprecationWarning: Numeric-style type codes are deprecated and will result in an error in the future. dtype('uint64')So the proposals to add
+ 1to the length would actually make the code wrong as it would prevent that DeprecationWarning from appearing. Not that it matters for future versions since the code has been removed, but that might be of interest for any OS distributions planning to "fix" older versions.I'm a tad confused by the lack of support for fixing this CVE? You've kinda left it hanging despite saying that
1.21is supported until June 23, 2023. https://numpy.org/neps/nep-0029-deprecation_policy.html#drop-scheduleReacted by Faisal@chtompki could you point to which part of the discussion above is confusing? I think the position of the NumPy team is quite clear. The process for disputing bogus CVEs is not transparent, we do not know why they have not withdrawn it.
Reacted by Rob TompkinsCurious, I've always found the CVE process workable when I've written them. Who is your CVE Numbering Authority (CNA)? Is it the 501(c)(3) NumFOCUS or did you guys go through the request directly from MITRE? If you went directly through MITRE, then you may have to file an appeal: https://www.cve.org/ResourcesSupport/AllResources/CNARules#section_9_appeals_process
Also, let me know if there's anything I can do to help. Would love to, if possible.
Curious, I've always found the CVE process workable when I've written them. Who is your CVE Numbering Authority (CNA)?
Well, nobody here has done this professional or even more than once. And maybe a point is that you are going through some CNA where the process surprisingly works easier?
We tried to tell MITR that it this is bogus, explaining why. They just asked for evidence without guidance how that would look like. So it is disputed, but thats it until someone explains how to "proof" that it is not a CVE.
Hm....I would ask who applied for the CVE Number? Can we not go back to them and ask them to un-apply? If not then we need to appeal to MITRE through their process. I'd be happy to help with that if possible
Hm....I would ask who applied for the CVE Number? Can we not go back to them and ask them to un-apply? If not then we need to appeal to MITRE through their process. I'd be happy to help with that if possible
Probably @Daybreak2019 who opened this issue. But they seem like a known bad actor, lots of bogus CVEs and no response after that anymore (see #18993 (comment) above).
This is the problem with the whole security circus - no accountability from anyone (neither the CVE submitter nor MITRE), and we get left with this mess.
Reacted by Ross Barnowski and Ryan Gibson
Reproducing code example:
Snippet:
Error message:
When we run our analysis tool on NumPy, an incomplete comparison problem was reported, see details below:
File: numpy/core/src/multiarray/descriptor.c
Function: _convert_from_str (line 1727 : 1740)
Optional call-path: PyArray_DescrAlignConverter -> _convert_from_any -> _convert_from_str
Details in description
NumPy/Python version information:
the main branch of NumPy