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

Gadgets, Clients, etc. #14

Closed
wants to merge 5 commits into from
Closed

Gadgets, Clients, etc. #14

wants to merge 5 commits into from

Conversation

mbechler
Copy link
Contributor

Sorry for the size of this and some unnecessary formatting changes.

A few more gadget RCE chains:

  • Hibernate5/Hibernate4
  • MyFaces
  • C3P0
  • net.sf.json-lib
    (beanutils you already got ;))

Non RCE Chains:

  • FileUpload (some potentially dangerous file ops still possible)
  • JavaSE (opening an JRMP listener/client, nice for bypassing application level filters)

In case you are interested I also just published my tooling that found these at
https://github.com/mbechler/serianalyzer (it's a static bytecode analyzer that traces native calls reachable by deserialization, many FPs of course)

And some explotitation tooling:

  • JRMP client/server
  • JSF ViewState
  • Jenkins CLI client (+ my POCs for CVE-2016-0788)
  • A JBoss/WildFly RMI client

@frohoff
Copy link
Owner

frohoff commented Mar 4, 2016

This is some great stuff and the analysis tool also looks fantastic.

As you said, it's a bit of a monster PR, so I was hoping we could tidy it up and trim/split it a bit, especially with some possibly non-trivial refactors on the horizon. Here's some immediate thoughts:

  • JDK Compatibility: There seem to be some dependencies on JDK8 classes (namely java.util.Base64). Can these be removed and make sure it builds against at least JDK7 (and JDK8)?
  • Git History: There's 3 merge commits in here. Perhaps this can call be rebased into a single commit against master?
  • License/Copyright: I'm no software license expert, but I don't think the "All rights reserved" (non-)licensing works inside an MIT licensed project. Can these be removed? I think Copyright (and definitely credit comments) are fine. Also see Support for gadget/exploit credits, CVEs, writeups #15
  • Unnecessary Changes: As you mentioned, there's a lot of whitespace/formatting changes to existing files, as well as many @SupressWarnings argument and $NON-NLS-1$ comment additions. Can these be removed/reverted to simplify the PR/merge? If some of these really do make sense, they should probably just be added to the whole codebase in a separate commit.
  • Tests: These things are certainly not easy to test, but with some larger refactors possibly coming down the pipe, it would be nice to know we weren't breaking gadgets/chains. Is it possible to build a test harness for some of these JNDI remote-load payloads and perhaps the commons-fileupload one?
  • Maven Profiles: I'm not sure if/why some of these maven profiles are necessary. Can you elaborate on the goals with this and/or if any of it could be simplified? For example, can the jenkins remoting jar be pulled down normally from this repo? Is there a way to make it play nicely with IDEs? Support dependency isolation and/or conflicting versions #10 may be relevant to this.

Again, this is some great work and I'm hoping we can get it pulled in without too much trouble. If you're short on time we can resubmit the PR against a different branch and I (or other contributors) can take a crack at getting pieces integrated.

Either way, thanks for the contribution, and please continue looking for more gadgets/chains and educating people about unsafe deserialization.

@mbechler
Copy link
Contributor Author

mbechler commented Mar 6, 2016

Superseded by #20

@mbechler mbechler closed this Mar 6, 2016
frohoff added a commit that referenced this pull request Mar 13, 2016
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

Successfully merging this pull request may close these issues.

2 participants