Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

new IPAddressBitsDivision leads to infinite loop #118

Closed
sirnple opened this issue Dec 5, 2023 · 12 comments
Closed

new IPAddressBitsDivision leads to infinite loop #118

sirnple opened this issue Dec 5, 2023 · 12 comments

Comments

@sirnple
Copy link

sirnple commented Dec 5, 2023

IPAddress version: 5.1.0
I have these code that can't finished in time:

    public void equalsFuzzerTest() {
        try {
            long tempLong = 5979084832330744063L;
            int tempInt = -1;
            IPAddressBitsDivision iPAddressBitsDivision = new IPAddressBitsDivision(tempLong, tempLong, tempInt, tempInt);
            boolean result = iPAddressBitsDivision.equals(new Object());
        } catch (Exception e) {
        }
    }

Is it a potential bug?

@seancfoley
Copy link
Owner

seancfoley commented Dec 7, 2023

That IPAddressBitsDivision contructor takes the arguments:
long value, long upperValue, int bitCount, int defaultRadix

The bitCount indicates the size of the division. The defaultRadix indicates the radix to use when printing the division.

So yes, as you have shown here, supplying a radix of -1 results in an infinite loop.

That is because of the function AddressDivisionBase.getDigitCount which does not work with a radix of -1 or 1, it will have an infinite loop with those arguments.

Theoretically, a radix can be negative, but in practice radices are generally 2 or larger.

There is no check in this library for a radix less than 2, but a radix less than 2 is an invalid argument that makes no logical sense. In this library, a radix should be 2 or larger, not negative, and not 1. So you can certainly fix your code here by avoiding calling any function with such a radix.

In a future release I can add a check to ensure that an exception is thrown when a radix is supplied that is not 2 or larger. I can also add a check in the bitcount argument, which should also not be negative, since that makes no sense as well.

@sirnple
Copy link
Author

sirnple commented Dec 8, 2023

Thanks for your reply, sir!👍

@rturner-edjuster
Copy link

@seancfoley For awareness, it seems that https://nvd.nist.gov/vuln/detail/CVE-2023-50570 has been raised for this issue (and of course as a result, the library is getting flagged). The issue, however, isn't very critical IMHO, and hasn't yet been a assigned a CVSS score. I think it's debatable if this should be raised as a CVE at all, instead of just a bug.

@seancfoley
Copy link
Owner

@rturner-edjuster I have become aware, and I agree with your assessment.

@seancfoley
Copy link
Owner

seancfoley commented Jan 3, 2024

@rturner-edjuster Not only that, I consider this bug very minor, something never encountered in production code anywhere.

That is because it makes no logical sense to pass in a radix less than 2 for an address division. It is simply not a valid argument, and nobody would see it as a valid argument. It doesn't strike me as a vulnerability at all, nor as a bug that needs any urgent attention.

Anyway, thanks for the notification.

@mike-jumper
Copy link

@seancfoley I reached out to GitHub, and they suggest reaching out to Mitre to reject the CVE:

Ohhh, ya that does look like it shouldn't have been assigned. We can go ahead and withdraw this instead if you think that's more appropriate. If so, I would also like to ask that someone involved with the project reach out to mitre to get the CVE rejected as well
https://cveform.mitre.org/

The https://cveform.mitre.org/ form allows you to send a request to update a CVE, with rejection being one of the types of updates available. See: github/advisory-database#3279 (comment)

@seancfoley
Copy link
Owner

Thanks @mike-jumper for the suggestion, I have submitted the request to mitre (CVE Request 1586075).

@richard-schoeller
Copy link

Any update on the request to withdraw?

@seancfoley
Copy link
Owner

There was no update to the mitre request. However, I noticed that on mitre and nist the vuln is now listed as disputed. The NIST page says "It is awaiting reanalysis".

I will be doing a new release of IPAddress in the next two to three weeks.

@seancfoley
Copy link
Owner

Fixed in version 5.4.1. Closing.

@noren95
Copy link

noren95 commented Jan 29, 2024

Regarding the CVE record - so this is not not considered a security issue?

@seancfoley
Copy link
Owner

@noren95 I do not consider it a security issue, and I don't see how it could be characterized as such.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants