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

Fix Null Pointer Issues #8580

Conversation

ZuhairORZaki
Copy link

In file: ComputerSet.java, class: ComputerSet, there is a method doDoCreateItem that there is a potential Null pointer dereference on the following line. This may throw an unexpected null pointer exception which, if unhandled, may crash the program.

// File: ComputerSet.java, Line: 305
public synchronized void doDoCreateItem(StaplerRequest req, StaplerResponse rsp,
                                           @QueryParameter String name,
                                           @QueryParameter String type) throws IOException, ServletException, FormException {
        ...

        // TODO type is probably NodeDescriptor.id but confirm
        Node result = NodeDescriptor.all().find(type).newInstance(req, formData);
        app.addNode(result);

        ...
    }

NodeDescriptor.all().find(type) calls the method find of class DescriptorExtensionList.

// File: DescriptorExtensionList.java, Line: 120
public D find(String fqcn) {
        return Descriptor.find(this, fqcn);
    }

This method then in turn calls find method of Descriptor class.

// File: Descriptor.java, Line: 1148
public static @CheckForNull <T extends Descriptor> T find(Collection<? extends T> list, String string) {
        T d = findByClassName(list, string);
        if (d != null) {
                return d;
        }
        return findById(list, string);
    }

The findById method referenced within can return a null value.

// File: Descriptor.java, Line: 1110
public static @CheckForNull <T extends Descriptor> T findById(Collection<? extends T> list, String id) {
        for (T d : list) {
            if (d.getId().equals(id))
                return d;
        }
        return null;
    }

Sponsorship and Support:

This work is done by the security researchers from OpenRefactory and is supported by the Open Source Security Foundation (OpenSSF): Project Alpha-Omega. Alpha-Omega is a project partnering with open source software project maintainers to systematically find new, as-yet-undiscovered vulnerabilities in open source code - and get them fixed – to improve global software supply chain security.

The bug is found by running the Intelligent Code Repair (iCR) tool by OpenRefactory and then manually triaging the results.

@welcome
Copy link

welcome bot commented Oct 9, 2023

Yay, your first pull request towards Jenkins core was created successfully! Thank you so much!

A contributor will provide feedback soon. Meanwhile, you can join the chats and community forums to connect with other Jenkins users, developers, and maintainers.

Copy link
Member

@NotMyFault NotMyFault left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The explanation lacks a practical issue, a proof of concept for the issue described, and lacks a proof of concept/real-world scenario to replicate that the changes proposed mitigate the issue described.

Similar to #8561, I value your interest in the Jenkins project, but it would help a lot to address the points above to get your PR over the finish line, rather than running static analysis only.

@NotMyFault NotMyFault added the spam This pull request is not a valid change proposal (e.g. vandalism, empty changes) label Oct 9, 2023
@ZuhairORZaki
Copy link
Author

Thank you for your feedback
I will be more careful from now on with providing a valid PoC for any issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
spam This pull request is not a valid change proposal (e.g. vandalism, empty changes)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants