ikkisoft/serialkiller

Look-Ahead Java Deserialization Library

serialkiller's Issues

Integrate with JEP-290 infrastructure

Hi there,
I like the idea of this library very much (thanks!), I would like to request the following feature to make it even more useful: Please add a implementation of https://docs.oracle.com/javase/9/docs/api/java/io/ObjectInputFilter.html

This way, one could use SerialKiller without having to change every ObjectInputStream call (which is kind of error-prone, because you can easily miss one).

Thanks in advance!

Proxy and InvocationHandler

Knowing that most of gadget relies on Proxy and InvocationHandler. Would it be possible to add those to the blacklist and look for implementation of InvocationHandler?

Package as agent or Byteman

Should be fine if it would be provided as a Java agent or Byteman package so as to inject functionality into the JDK ObjectInputStream

SerialKiller creates excessive logfiles if logging enabled

The following code in SerailKiller's constructor is problematic, as it will create a new FileHandler (along with a new file) for each instance. It should probably be noted in the readme that this option is only recommended for single instances, and when profiling is enabled.

        if (config.isLogging()) {
            Handler fileHandler = new FileHandler(config.logFile(), true);
            LOGGER.addHandler(fileHandler);
            LOGGER.setLevel(Level.ALL);
        }

I'm thinking this code may be dropped entirely, and instead be left to user configuration, using the java.util.logging.config.file system property or lib/logging.propertis as described in the LogManager API doc.

Best regards,

--
Harald K

Was GPL license intentional?

Seems like a potentially interesting and helpful library, but I'd expect this to be Apache or at least LGPL licensed. Do you really intend this to be GPL? This will prevent a vast majority of developers from being able to use this

Cache for RegExp to improve performance

With the current implementation, each regexp lookup involves filesystem access

$ inotifywait -mr ./
./ OPEN serialkiller.conf
./ ACCESS serialkiller.conf
./ CLOSE_NOWRITE,CLOSE serialkiller.conf

To improve performances, we could create a cache for all blacklists/whitelists - dropped whenever configuration reloads. Potentially, we could also make that cache static and/or thread-local and dependent on the filename, so that two SerialKillers with the same config share the cache

Request for source code merging

Hi Luca,

I'd like to incorporate some of the code in https://github.com/wsargent/paranoid-java-serialization/ (the blacklist and whitelist). It won't be a straight mapping -- right now it looks like:

    protected Class<?> resolveClass(ObjectStreamClass desc)
            throws IOException, ClassNotFoundException
    {
        String name = desc.getName();
        logger.fine("resolveClass: resolving " + name);

        // From https://github.com/ikkisoft/SerialKiller by [email protected]

        //Enforce blacklist
        for (Pattern blackPattern : blacklistPatterns) {
            Matcher blackMatcher = blackPattern.matcher(name);
            if (blackMatcher.find()) {
                logger.warning("resolveClass: rejecting blacklisted class " + name);
                String msg = "[!] Blocked by blacklist '"
                        + blackPattern.pattern() + "'. Match found for '" + name + "'";
                throw new InvalidClassException(msg);
            }
        }

        //Enforce whitelist if it exists.
        if (! whitelistPatterns.isEmpty()) {
            boolean safeClass = false;
            for (Pattern whitePattern: whitelistPatterns) {
                Matcher whiteMatcher = whitePattern.matcher(name);
                if (whiteMatcher.find()) {
                    safeClass = true;
                    break;
                }
            }

            if (!safeClass) {
                logger.warning("resolveClass: rejecting class " + name + " not found in whitelist.");
                String msg = "[!] Blocked by whitelist. No match found for '" + name + "'";
                throw new InvalidClassException(msg);
            }
        }

        try {
            logger.fine("resolveClass: accepting class " + name);
            return Class.forName(name, false, latestUserDefinedLoader());
        } catch (ClassNotFoundException ex) {
            Class<?> cl = primClasses.get(name);
            if (cl != null) {
                return cl;
            } else {
                throw ex;
            }
        }
    }

It's static, and doesn't use any additional classes (since it has to be in a hacked java.io.ObjectInputStream itself).

Is this okay? Are there any changes or attribution you'd like?

Use commons-configuration2 instead of deprecated commons-configuration

Use commons-configuration2 instead of deprecated commons-configuration

Wrong/missing blacklist entries

Not that I would count on the blacklist, but I think your entry for the spring vector is wrong.

The piece of code that gives the arbitrary method invocation that is needed is

org.springframework.core.SerializableTypeWrapper$MethodInvokeTypeProvider

and not the ObjectFactory (that just conveniently supplies the instance to call upon). Certainly does not hurt to forbid that, too, but the MethodInvokeTypeProvider alone can ruin your day.

New Ideas for features

Hi,

I've recently written a similar library. Since it does not make a lot of sense to have two similar open source libraries I've ported the additional features to SerialKiller.

These features are:

• More flexibility for blacklist and whitelist rules (AND-, NOT-, OR-expressions, etc.). The API is extensible to allow additional rule types (I have a few ideas for future development: e.g. the class to be deserialized must implement Interface x, ...). This makes it a lot easier to implement a restrictive whitelist.
• Allow creation of a deserialization policy at runtime. IMHO this makes code more readable since the policy is not in a separate file. Also, currently SerialKiller seems to not accept file paths relative to the classpath - that may be a problem for many developers/organizations.
• More flexibility when handling deserialization policy violations: log the violation, throw an exception or implement a custom handler.

I've pushed the draft version here: https://github.com/ettisan/SerialKiller/tree/runtime_configuration

I'd appreciate feedback regarding the API, etc. If you think that the features should be part of the mainline SerialKiller let me know - I would then continue to develop this branch towards release quality.

Threading issue/race condition

Hi!

Thanks for creating this!

However, during testing I've found some bugs. The static variables in SerialKiller that are assigned in constructor may cause serious race condition and confusing results, if using multiple configurations and multiple threads.

Consider the following test case:

@Test(expected = InvalidClassException.class)
public void testThreadIssue() throws Exception {
    ByteArrayOutputStream bytes = new ByteArrayOutputStream();

    try (ObjectOutputStream stream = new ObjectOutputStream(bytes)) {
        stream.writeObject(42);
    }

    try (ObjectInputStream stream = new SerialKiller(new ByteArrayInputStream(bytes.toByteArray()), "src/test/resources/blacklist-all.conf")) { // Blacklist: .*
        // Create a dummy SK with different config
        new SerialKiller(new ByteArrayInputStream(bytes.toByteArray()), "src/test/resources/whitelist-all.conf"); // No blacklist and whitelist: .*

        stream.readObject(); // Now passing...

        fail("All should be blacklisted");
    }
}

PS: I'll be back with a PR later, fixing this. :-)

Best regards,

--
Harald K

Add the ability to specify the expected class in the code

Currently when instantiating the SerialKiller you need to specify the configuration file.
NameOfExpectedClass expectedClass = (NameOfExpectedClass) new SerialKiller(inputstream, "/location/to/serialkiller.conf", NameOfExpectedClass.class ).readObject()

Also if the previously requested feature was added to include a default blacklist you could even have
NameOfExpectedClass expectedClass = (NameOfExpectedClass) new SerialKiller(inputstream, NameOfExpectedClass.class ).readObject()

This would add the "NameOfExpectedClass" to the whitelist so if the developer tried to use this to read a blacklisted class it would still fail ( as blacklist takes priority over whitelist ).

Adding the default blacklist and giving the developer the ability to specify in code the expected class. I think would make the library easier to use and increase adoption.
Currently modifying an xml configuration file that needs to be kept on the filesystem adds extra overhead and limits adoption for environments where ensuring the file is on the filesystem or modifying it is difficult or impossible.

I'm happy to raise a pull request to add the above functionality.

Exception does not distinguish between blacklist and non-whitelist blocking

It is sometimes useful for client code to be able to distinguish between a blacklist match and a white-list non-match, as they may require quite different actions (ie. edit the whitelist or rewrite code to not rely on unsafe classes, or worse...).

I suggest making the words "blacklist" and "non-whitelist" part of the exception message, as a low impact solution.

InvalidClassException thrown does not include the class name of the blocked class

It is useful for client code to be able to log extra information when a class is blocked from deserialization, and thus needs to know the class name that was blocked.

The InvalidClassException class has a (public...) field classname that can be used for this purpose, but it is currently not initialized (though it is part of the message string).

Publish the library to Maven central

Publish to Maven so that people can absorb and stay up to date.

Requested by https://twitter.com/nahsra and https://www.reddit.com/user/vytah

java.nio.file.NoSuchFileException: \tmp\serialkiller.log.lck

getting an error : java.nio.file.NoSuchFileException: \tmp\serialkiller.log.lck

Which License applies?

As a fix for #1 you changed the licence to Dual-License GPL & Apache License 2.0.

What is unclear (at least to me) is which license applies under which circumstances?

• Do all licenses apply equally?
• Can the user/developer choose which license should apply?
• What are the conditions that allow me to use the software under Apache License?

Could you please clearify this? Right now the unclear license is a major obstacle for using SerialKiller.

Extend black-listing/white-listing check to support Interfaces

It may be interesting to allow users to specify interfaces and block/allow classes implementing those interfaces.

It would require to modify the black-listing/white-listing check, adding some logic around public Class<?>[] getInterfaces().

Not sure if we want to add a specific notation for interfaces in the config file, or simply add those items as the current class names.

Also, a class may implement multiple interfaces thus we need to figure out a good strategy.

Include Blacklist in Library

Though the blacklist is in the Git repo it is not included in the release JARs. Right now, a project that wants to include SerialKiller has to:

• include the Jar (e.g. Maven)
• copy the default configuration file and customize it

This is problematic since when the blacklist in the git repo is changed to include more vulnerable classes they are most likely not transferred to the config file.

I think it would therefore be better to include the blacklist into the JARs. By default the blacklist should be applied to all SerialKiller instances. This way, when the blacklist changes only the Jar has to be updated - the custom configuration file does not have to be modified.

I'm willing to implement this. Please give me a heads up as if you would want to accept such a pull request.

ClassNotFoundException when using our own packages in the whitelist

java.lang.IllegalAccessError when instantiating SerialKiller

hi there,

when executing next code I get an error when instantiating SerialKiller class., do you know which is the problem ?

buffer = new ByteArrayInputStream(Base64.decodeBase64(coded.getBytes())); // ObjectInputStream ois = new SerialKiller(buffer, "serialkiller.conf");

the stacktrace:
ERROR [io.undertow.request] (default task-34) UT005023: Exception handling request to /RemoteProcessorWeb/: java.lang.BootstrapMethodError: java.lang.IllegalAccessError: at org.nibblesec.tools.SerialKiller.<init>(SerialKiller.java:59) at com.nte.anthema.framework.text.ObjectToStringCodec.decode(ObjectToStringCodec.java:68) at com.nte.anthema.framework.command.RemoteCommandProcessServlet.decodeString(RemoteCommandProcessServlet.java:198) at com.nte.anthema.framework.command.RemoteCommandProcessServlet.doPost(RemoteCommandProcessServlet.java:59) at javax.servlet.http.HttpServlet.service(HttpServlet.java:707) at javax.servlet.http.HttpServlet.service(HttpServlet.java:790) at io.undertow.servlet.handlers.ServletHandler.handleRequest(ServletHandler.java:85) at io.undertow.servlet.handlers.FilterHandler$FilterChainImpl.doFilter(FilterHandler.java:129) at com.nte.anthema.filter.AuthFilter.doFilter(AuthFilter.java:130) at io.undertow.servlet.core.ManagedFilter.doFilter(ManagedFilter.java:61) at io.undertow.servlet.handlers.FilterHandler$FilterChainImpl.doFilter(FilterHandler.java:131) at io.undertow.servlet.handlers.FilterHandler.handleRequest(FilterHandler.java:84) at io.undertow.servlet.handlers.security.ServletSecurityRoleHandler.handleRequest(ServletSecurityRoleHandler.java:62) at io.undertow.servlet.handlers.ServletDispatchingHandler.handleRequest(ServletDispatchingHandler.java:36) at org.wildfly.extension.undertow.security.SecurityContextAssociationHandler.handleRequest(SecurityContextAssociationHandler.java:78) at io.undertow.server.handlers.PredicateHandler.handleRequest(PredicateHandler.java:43) at io.undertow.servlet.handlers.security.SSLInformationAssociationHandler.handleRequest(SSLInformationAssociationHandler.java:131) at io.undertow.servlet.handlers.security.ServletAuthenticationCallHandler.handleRequest(ServletAuthenticationCallHandler.java:57) at io.undertow.server.handlers.DisableCacheHandler.handleRequest(DisableCacheHandler.java:33) at io.undertow.server.handlers.PredicateHandler.handleRequest(PredicateHandler.java:43) at io.undertow.security.handlers.AuthenticationConstraintHandler.handleRequest(AuthenticationConstraintHandler.java:53) at io.undertow.security.handlers.AbstractConfidentialityHandler.handleRequest(AbstractConfidentialityHandler.java:46) at io.undertow.servlet.handlers.security.ServletConfidentialityConstraintHandler.handleRequest(ServletConfidentialityConstraintHandler.java:64) at io.undertow.servlet.handlers.security.ServletSecurityConstraintHandler.handleRequest(ServletSecurityConstraintHandler.java:59) at io.undertow.security.handlers.AuthenticationMechanismsHandler.handleRequest(AuthenticationMechanismsHandler.java:60) at io.undertow.servlet.handlers.security.CachedAuthenticatedSessionHandler.handleRequest(CachedAuthenticatedSessionHandler.java:77) at io.undertow.security.handlers.NotificationReceiverHandler.handleRequest(NotificationReceiverHandler.java:50) at io.undertow.security.handlers.AbstractSecurityContextAssociationHandler.handleRequest(AbstractSecurityContextAssociationHandler.java:43) at io.undertow.server.handlers.PredicateHandler.handleRequest(PredicateHandler.java:43) at org.wildfly.extension.undertow.security.jacc.JACCContextIdHandler.handleRequest(JACCContextIdHandler.java:61) at io.undertow.server.handlers.PredicateHandler.handleRequest(PredicateHandler.java:43) at io.undertow.server.handlers.PredicateHandler.handleRequest(PredicateHandler.java:43) at io.undertow.servlet.handlers.ServletInitialHandler.handleFirstRequest(ServletInitialHandler.java:292) at io.undertow.servlet.handlers.ServletInitialHandler.access$100(ServletInitialHandler.java:81) at io.undertow.servlet.handlers.ServletInitialHandler$2.call(ServletInitialHandler.java:138) at io.undertow.servlet.handlers.ServletInitialHandler$2.call(ServletInitialHandler.java:135) at io.undertow.servlet.core.ServletRequestContextThreadSetupAction$1.call(ServletRequestContextThreadSetupAction.java:48) at io.undertow.servlet.core.ContextClassLoaderSetupAction$1.call(ContextClassLoaderSetupAction.java:43) at io.undertow.servlet.api.LegacyThreadSetupActionWrapper$1.call(LegacyThreadSetupActionWrapper.java:44) at io.undertow.servlet.api.LegacyThreadSetupActionWrapper$1.call(LegacyThreadSetupActionWrapper.java:44) at io.undertow.servlet.api.LegacyThreadSetupActionWrapper$1.call(LegacyThreadSetupActionWrapper.java:44) at io.undertow.servlet.api.LegacyThreadSetupActionWrapper$1.call(LegacyThreadSetupActionWrapper.java:44) at io.undertow.servlet.api.LegacyThreadSetupActionWrapper$1.call(LegacyThreadSetupActionWrapper.java:44) at io.undertow.servlet.handlers.ServletInitialHandler.dispatchRequest(ServletInitialHandler.java:272) at io.undertow.servlet.handlers.ServletInitialHandler.access$000(ServletInitialHandler.java:81) at io.undertow.servlet.handlers.ServletInitialHandler$1.handleRequest(ServletInitialHandler.java:104) at io.undertow.server.Connectors.executeRootHandler(Connectors.java:202) at io.undertow.server.HttpServerExchange$1.run(HttpServerExchange.java:805) at java.util.concurrent.ThreadPoolExecutor.runWorker(Unknown Source) at java.util.concurrent.ThreadPoolExecutor$Worker.run(Unknown Source) at java.lang.Thread.run(Unknown Source) Caused by: java.lang.IllegalAccessError: ... 51 more

Can you please create new maven release for 0.5 release

Can you please create new maven release for 0.5 release