dxbjavid commented on PR #403: URL: https://github.com/apache/commons-net/pull/403#issuecomment-4886043582
you're right, good catch. the single-arg Class.forName still initialises the class, so the static initialiser ran before isAssignableFrom ever looked at the type. moving the check earlier only stopped the constructor, not the static init. i've switched to the three-arg Class.forName(key, false, loader) so the class is loaded without being initialised. it only gets initialised when a genuine FTPFileEntryParser is actually constructed. the test now asserts the static initialiser doesn't run, and it fails against the initialising forName (expected false, was true), so it guards the real behaviour now. on how much this actually matters: it's narrow. the class has to already be on the client's classpath and have a visible side effect in its static block, and the trigger is the SYST reply during listFiles auto-detection, so a malicious or intercepted server picks which existing class gets initialised. no arbitrary code beyond what's already shipped. more defence-in-depth than a sharp vuln, but loading without init is basically free and keeps remote input from initialising unrelated classes, so it seemed worth tightening. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
