Costin Manolache wrote:
I will start looking at the doPrivilege block. One thing that I have found is most Tomcat's doPrivilege block contains more operations that they should. Minimizing what we put inside the doPrivilege is very important. As an example (need to be optimized), I made some change to the WebappClassLoader (see the diff.txt).AFAIK, the most important check is doPriviledged(). What we need to look for is if any of those blocks could be used by untrusted code to do something.The second very important check is the facades - making sure untrusted code can't get access to the real objects. We should also make sure that the facades are not reused or are reused only inside a context. It is nice to review all code for security and bugs ( and general quality ) - but IMO it is more important to review first the critical areas.
Other areas we can look first are:The ClassLoader shouldn't create any major problems ( at least in delegating mode - in non delegating mode we can only trust the servlet experts that they did the resarch and can guarantee there's no security implications. I haven't heard anything convincing on this, but they should have this knowledge - otherwise it wouldn't be in the spec :-)
- Admin Tool. Is the tool secure enougth?
- Invoker/Defaut Servlet ;-)
- The code Jasper generates. This is one place where facade will get accessed.
-- Jeanfrancois
Costin Bob Herrmann wrote:FYI, Just to start off, I am going to review these classes. If someone else also reviews them, thats probably a good thing... # classes, package name 17 o.a.c.deploy 9 o.a.c.users 44 o.a.c.* 34 o.a.jk.* 15 j.s.http Briefly, I am going to look for - How/if a ClassLoader is used - privilege blocks (are they small, use trusted values) - look for non-final static variables (can they be abused) - can methods/fields be made private? - are mutable objects returned to caller (especially arrays) think about returning clones - non final equals/hashcode methods? (accessing sensitive stuff?) - Serializable (exposes private stuff?) Does anyone publish a security checklist list like this? Blah Blah, -bob On Tue, 2002-10-08 at 16:36, Jean-Francois Arcand wrote:Hi, I'm looking to do a Security Audit on the current Tomcat 5.0 codebase. I would like to collect as more as information as where you think I should look at (code, security hole, etc.). I'm planning to do the audit using the default SecurityManager. Rigth now, I have started looking at: - doPrivilege blocks. Are they small enough? Can they be reduced? - JSP generated code. Are they secure? Can a malicious app uses the code to access o.a.catalina code? - Is catalina.policy restricted enough? - Is our Classloader secure? Any direction/ideas/recommendations will be appreciated. Thanks, -- Jeanfrancois -- To unsubscribe, e-mail: <mailto:[EMAIL PROTECTED]> For additional commands, e-mail: <mailto:[EMAIL PROTECTED]>
Index: WebappClassLoader.java
===================================================================
RCS file:
/home/cvs/jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/loader/WebappClassLoader.java,v
retrieving revision 1.6
diff -u -r1.6 WebappClassLoader.java
--- WebappClassLoader.java 5 Sep 2002 11:38:59 -0000 1.6
+++ WebappClassLoader.java 9 Oct 2002 19:23:07 -0000
@@ -154,16 +154,16 @@
protected class PrivilegedFindResource
implements PrivilegedAction {
- private String name;
+ private int filePointer;
private String path;
- PrivilegedFindResource(String name, String path) {
- this.name = name;
+ PrivilegedFindResource(int filePointer, String path) {
+ this.filePointer = filePointer;
this.path = path;
}
public Object run() {
- return findResourceInternal(name, path);
+ return findResourceInternal(filePointer, path);
}
}
@@ -895,13 +895,7 @@
ResourceEntry entry = (ResourceEntry) resourceEntries.get(name);
if (entry == null) {
- if (securityManager != null) {
- PrivilegedAction dp =
- new PrivilegedFindResource(name, name);
- entry = (ResourceEntry)AccessController.doPrivileged(dp);
- } else {
- entry = findResourceInternal(name, name);
- }
+ entry = findResourceInternal(name, name);
}
if (entry != null) {
url = entry.source;
@@ -1479,13 +1473,7 @@
ResourceEntry entry = null;
- if (securityManager != null) {
- PrivilegedAction dp =
- new PrivilegedFindResource(name, classPath);
- entry = (ResourceEntry)AccessController.doPrivileged(dp);
- } else {
- entry = findResourceInternal(name, classPath);
- }
+ entry = findResourceInternal(name, classPath);
if ((entry == null) || (entry.binaryContent == null))
throw new ClassNotFoundException(name);
@@ -1560,6 +1548,23 @@
}
+ /**
+ * Find specified resource in local repositories. This block
+ * will execute under an AccessControl.doPrivilege block.
+ *
+ * @return the loaded resource, or null if the resource isn't found
+ */
+ private ResourceEntry findResourceInternal(int filePointer, String path){
+ ResourceEntry entry = new ResourceEntry();
+ try {
+ entry.source = getURL(new File(files[filePointer], path));
+ entry.codeBase = entry.source;
+ } catch (MalformedURLException e) {
+ return null;
+ }
+ return entry;
+ }
+
/**
* Find specified resource in local repositories.
@@ -1603,13 +1608,14 @@
// Note : Not getting an exception here means the resource was
// found
- entry = new ResourceEntry();
- try {
- entry.source = getURL(new File(files[i], path));
- entry.codeBase = entry.source;
- } catch (MalformedURLException e) {
- return null;
- }
+ if (securityManager != null) {
+ PrivilegedAction dp =
+ new PrivilegedFindResource(i, path);
+ entry = (ResourceEntry)AccessController.doPrivileged(dp);
+ } else {
+ entry = findResourceInternal(i, path);
+ }
+
ResourceAttributes attributes =
(ResourceAttributes) resources.getAttributes(fullPath);
contentLength = (int) attributes.getContentLength();
-- To unsubscribe, e-mail: <mailto:[EMAIL PROTECTED]> For additional commands, e-mail: <mailto:[EMAIL PROTECTED]>
