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

Cleaned up version of #14 #20

Merged
merged 25 commits into from
Mar 13, 2016
Merged

Cleaned up version of #14 #20

merged 25 commits into from
Mar 13, 2016

Conversation

mbechler
Copy link
Contributor

@mbechler mbechler commented Mar 6, 2016

Took the time and cleaned that stuff up quite a bit.

With regards to the issues raised

  • JDK Compatibility: Base64 now handled with commons-codec
  • Git History: rebased against master, finer-grained and cleaner commits
  • License/Copyright: My fault, these were just leftovers from my template. MIT is fine. Do you need me to explicitely state that in the source?
  • Tests: Now near complete coverage. Built some infrastucture for that (a bit hackish in some cases).
  • Maven profiles: My thinking was that dependencies were getting a bit out of hand, so that saved a few MBs on the final JAR and dependency download when one didn't need some of the more exotic stuff. Also did't see that jenkins repository, thanks. The remaining profiles are build-time switches for bundling different incompatible library versions. These could be removed but without a mechanism for that then one would have to put the alternatives manually on the classpath (testing with the different alternatives would also be harder).

With regards to #10, maybe an solution could also allow doing that switch at runtime and bundling multiple alternatives.
I'd probably go with the reflection based approach - in cases where it turns out to be necessary - (like I did in the hibernate payload) as I don't think that build time conflicts will be actually that common and a multi module variant might lead to some duplication.

@mbechler mbechler mentioned this pull request Mar 6, 2016
@frohoff
Copy link
Owner

frohoff commented Mar 7, 2016

I'll have to take a closer look, but so far this looks great. Thanks a ton for putting in the extra work.

Regarding the licenses: AFAIK it should just inherit the license of the project as a whole as long as there's no other licenses (or "All rights reserved") attached. Make sure your name is somewhere in any code you want to be credited with (gadgets/chains and/or exploits).

Regarding the maven profile stuff: This is probably as good as it will get without a larger overhaul of how this is handled (as per #10), which is at the top of my list to sort out ASAP. Suggestions definitely welcome as usual.

We should also make sure that any exploitable stuff has been reported to the project team before we release it here, and ideally reference a CVE in the exploit code if one exists.

The test stuff you added actually looks great and is very similar to what I had in mind.

I'll probably have a few questions/comments on the commits in the PR, but other than that this looks great.

Thanks again.

@@ -0,0 +1,17 @@
/**
* © 2016 AgNO3 Gmbh & Co. KG
* All right reserved.
Copy link
Owner

Choose a reason for hiding this comment

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

Looks like one more slipped in ;-)

* @author mbechler
*
*/
public class JenkinsCLI {
Copy link
Owner

Choose a reason for hiding this comment

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

A description of how this works would be ideal here, especially if/how it interoperates with other ysoserial components. Is this part of CVE-2016-0788 or something separate?

@frohoff
Copy link
Owner

frohoff commented Mar 7, 2016

Some short descriptions/notes on how the new exploit package and JRMP stuff works would be helpful.

@mbechler
Copy link
Contributor Author

mbechler commented Mar 9, 2016

Added some docs for the tooling. That exploit package isn't new, is it? ;) Figured that would be the correct place for tooling delivering playloads.

With regards to JBoss/Wildfly:
Judging from
http://lists.jboss.org/pipermail/wildfly-dev/2015-November/004581.html
and
https://access.redhat.com/solutions/2045023
they are aware of the problem in general, and as far as my (limited) research goes I haven't found anything that would be directly exploitable in just a base installation (very restrictive classloaders on the exported stuff, also authentiction needed). So this is mainly a tool for people that may want to test their servers.

@h3xstream
Copy link
Contributor

/cc @frohoff
I have a proposition to clean the build while handling the library isolation.

Here is a POC that separate each gadget in modules and would allow simple libraries isolation.
https://github.com/GoSecure/ysoserial/tree/feature/gradle-build

Gradle will bring two main advantages to the projet

  • It will bring minimal configuration for new gadget (compare to XML file required by Maven)
  • Simplify the packaging for library isolation (See packaging task)

The POC only illustrate the intended structure and packaging. Some code would be needed to dynamically
(See bootstrap class).

Let me know if you have questions.

Add exploit and test code for JRMP reverse connect remote classloading.
frohoff added a commit that referenced this pull request Mar 13, 2016
@frohoff frohoff merged commit 4b798a1 into frohoff:master Mar 13, 2016
@frohoff
Copy link
Owner

frohoff commented Mar 13, 2016

Thanks again for the contribution and sorry it took so long. Going to try to accept a couple other PRs and do some cleanup before releasing.

@frohoff
Copy link
Owner

frohoff commented Mar 14, 2016

Seeing some intermittent flakiness with the PayloadsTest for JRMPClient failing intermittently and was hoping you could take a look if/when you get a chance. It failed a couples times for me locally but now it seems to be passing consistently, though it's still failing on travis-ci intermittently. Race condition?

@mbechler
Copy link
Contributor Author

So far haven't been able to reproduce locally. One potential issue I can think of is that, depending on test execution order, the java.rmi.server.useCodebaseOnly property (currently set in Gadgets) is not always set before sun.rmi.server.MarshalInputStream is loaded. Would probably be better to set this directly on the test VM in the pom (also it's not needed otherwise).

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.

3 participants