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

Detect if entity objects are being returned by controllers in Spring #454

Closed
karanb192 opened this issue Feb 9, 2019 · 9 comments
Closed
Assignees
Labels
enhancement New feature or improvement to existing detector.
Milestone

Comments

@karanb192
Copy link
Contributor

In Spring, developers often tend to return the entity object as response in controllers which may reveal sensitive information from the DB which wasn't really needed. Instead DTO (Data transfer object) should be returned.

@karanb192
Copy link
Contributor Author

karanb192 commented Feb 9, 2019

This is an example of vulnerable code.

        @GetMapping("/sample/api")
	public SampleEntityClass sampleMethod(Principal principal) {
                Query query = getEntityManager().createQuery("select c from SampleTable c");
                SampleEntityClass entityObject = query.getResultList().get(0);
                return entityObject;
	}

@karanb192
Copy link
Contributor Author

karanb192 commented Feb 9, 2019

I'd like to add a custom detector for this vulnerability.
@h3xstream Views?

@h3xstream h3xstream added the enhancement New feature or improvement to existing detector. label Feb 11, 2019
@h3xstream
Copy link
Member

Interesting idea to find information leakage, the same detector could look at parameter for update operation to find potential mass assignment.

@GetMapping("/sample/api")
public void update(SampleEntityClass updateEntity) {
  ...
}

@karanb192
Copy link
Contributor Author

Will cover this as well. Entity objects shouldn't be accept as request parameters as well. Instead Request DTOs should be used.

@karanb192
Copy link
Contributor Author

I'll start working on this.
@h3xstream please assign this issue to me.

@h3xstream
Copy link
Member

h3xstream commented Feb 15, 2019

@karanb192 Cool
The challenge I see is that with can't just use Inheritance API to check if the class inherit from a specific class. We need to look at all the subclasses for the annotation @entity .

  • javax.persistence.Entity (JPA)
  • javax.jdo.spi.PersistenceCapable (JDO)
  • org.springframework.data.mongodb.core.mapping.Document (Document database)

@karanb192
Copy link
Contributor Author

karanb192 commented Mar 3, 2019

Hi @h3xstream
I've raised a PR. I check for all the subclasses for above 3 annotations recursively. Please have a look - https://github.com/find-sec-bugs/find-sec-bugs/pull/457

@h3xstream
Copy link
Member

PR integrated

@h3xstream h3xstream added this to the version-1.9.0 milestone Mar 28, 2019
@karanb192
Copy link
Contributor Author

Hi @h3xstream
I'd like to extend this check for collections of entity objects. What say?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or improvement to existing detector.
Projects
None yet
Development

No branches or pull requests

2 participants