db-derby-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Knut Anders Hatlen (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (DERBY-6619) After silently swallowing SecurityExceptions, Derby can leak class loaders
Date Mon, 25 Aug 2014 08:52:58 GMT

    [ https://issues.apache.org/jira/browse/DERBY-6619?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14108905#comment-14108905
] 

Knut Anders Hatlen commented on DERBY-6619:
-------------------------------------------

Thanks, Dag. The patch and the test case look good to me. +1 to commit.

I'm wondering if we may want to make a tiny refinement of the if statement in a follow-up.
We could change it from

{code}
                    if (cl == ClassLoader.getSystemClassLoader()) {
{code}

to

{code}
                    if (cl == getClass().getClassLoader() ||
                            cl == Thread.class.getClassLoader()) {
{code}

That would be two changes from the original:

1) Check if the context class loader is the same as the loader of the SingletonTimerFactory
class (that is, the class loader from which Derby classes are loaded). Even if this isn't
the same as the system class loader, it would be safe to skip the workaround for DERBY-3745,
since the class loader for the Derby classes would not be eligible for garbage collection
until the engine is shut down anyway. This would prevent some more false positives from being
reported in derby.log. I'd assume this is a kind of false positive that could easily occur
in an application server environment where the Derby classes are not found on the system class
loader.

2) Check against Thread.class.getClassLoader() instead of ClassLoader.getSystemClassLoader().
Normally, those two checks would be identical. If however the setup is such that they aren't
identical, we'd still know that the class loader actually used for system classes such as
the Thread class would have to stay in memory for as long as the timer thread is alive, regardless
of the context class loader of the thread, so the workaround for DERBY-3745 could be skipped
if it's the same as the context class loader. The check would be a bit more specific this
way.

Not sure about #2, but at least #1 sounds worthwhile. FWIW, the new SecureServerTest passes
with the suggested additional changes too.

> After silently swallowing SecurityExceptions, Derby can leak class loaders
> --------------------------------------------------------------------------
>
>                 Key: DERBY-6619
>                 URL: https://issues.apache.org/jira/browse/DERBY-6619
>             Project: Derby
>          Issue Type: Bug
>          Components: Services
>            Reporter: Rick Hillegas
>            Assignee: Dag H. Wanvik
>             Fix For: 10.11.1.2, 10.12.0.0
>
>         Attachments: derby-6619-2.diff, derby-6619.diff, derby-6619.status, derby-6619b.diff,
derby-6619c.diff, derby.log, system-loader.diff
>
>
> As part of the fix for DERBY-3745, Derby silently swallows security exceptions and leaks
class loaders. This can give rise to denial-of-service attacks. At a minimum, Derby should
report the swallowed exceptions so that the security policy can be corrected and the application
can be hardened against this attack. The swallowing occurs at these locations:
> {noformat}
> org.apache.derby.impl.services.timer.SingletonTimerFactory run Catch java.lang.SecurityException
0 line 175
> org.apache.derby.impl.services.timer.SingletonTimerFactory run Catch java.lang.SecurityException
1 line 158
> {noformat}



--
This message was sent by Atlassian JIRA
(v6.2#6252)

Mime
View raw message