Skip to content

Fix critical security vulnerabilities (XXE, outdated deps, info leak)#2

Open
devin-ai-integration[bot] wants to merge 1 commit into
masterfrom
devin/1776159731-security-fixes
Open

Fix critical security vulnerabilities (XXE, outdated deps, info leak)#2
devin-ai-integration[bot] wants to merge 1 commit into
masterfrom
devin/1776159731-security-fixes

Conversation

@devin-ai-integration
Copy link
Copy Markdown

Summary

Security audit of the codebase identified several vulnerabilities. This PR fixes the critical and medium-severity issues:

Critical — XXE Vulnerability (CWE-611):
XmlBeanDefinitionReader.doLoadBeanDefinitions() created a DocumentBuilderFactory without disabling external entity processing. An attacker who controls the XML input could read arbitrary files, perform SSRF, or cause DoS. Fixed by disabling DOCTYPE declarations, external general/parameter entities, external DTD loading, XInclude, and entity reference expansion.

Critical — Outdated Dependencies with Known CVEs:

  • JUnit 4.74.13.2 (CVE-2020-15250: temp directory info leak)
  • cglib-nodep 2.1_33.3.0 (severely outdated, no longer maintained at old version)
  • aspectjweaver 1.6.111.9.21

Medium — Information Leak via e.printStackTrace():
BeanDefinition.setBeanClassName() swallowed ClassNotFoundException and printed the stack trace to stderr, leaking internal paths. Replaced with proper exception propagation (IllegalStateException).

Medium — Input Validation:
Added null/empty check for class names in BeanDefinition.setBeanClassName().

Medium — Resource Leak:
InputStream in XmlBeanDefinitionReader.doLoadBeanDefinitions() was not closed if parsing threw an exception. Wrapped in try-finally.

Build Compatibility:

  • Upgraded Java source/target from 1.6 to 1.8 (1.6 is no longer supported by modern JDKs)
  • Added maven-surefire-plugin 3.2.5 with --add-opens for Java 17 module system compatibility

All fixes applied to both src/ and src+/ copies of affected files.

Review & Testing Checklist for Human

  • Verify the XXE fix in XmlBeanDefinitionReader.doLoadBeanDefinitions() — confirm DocumentBuilderFactory features are set correctly before any parsing occurs
  • Verify dependency versions are acceptable for your project (JUnit 4.13.2, cglib-nodep 3.3.0, aspectjweaver 1.9.21)
  • Verify Java 1.8 source/target is acceptable (was 1.6, which doesn't compile on Java 17+)
  • Run mvn clean test to confirm 9/10 tests pass (the 1 failure — testPostBeanProcessor — is a pre-existing bug: OutputServiceImpl lacks the helloWorldService field that tinyioc-postbeanprocessor.xml tries to inject)
  • If you need Java 1.6 compatibility, this PR would need adjustment — but Java 1.6 is EOL and cannot be compiled on modern JDKs

Notes

  • No hardcoded secrets, SQL injection, CORS issues, debug endpoints, or missing auth checks were found — the project is a pure IoC container with no web/HTTP layer.
  • The setAccessible(true) usage in AutowireCapableBeanFactory is noted but not changed — it's inherent to how IoC property injection works via reflection.
  • testPostBeanProcessor failure is pre-existing and unrelated to this PR.

Link to Devin session: https://app.devin.ai/sessions/975441d382b1410ea460df014f50e55f
Requested by: @scoldfield

- Fix XXE vulnerability in XmlBeanDefinitionReader by disabling external
  entity processing in DocumentBuilderFactory (CWE-611)
- Upgrade outdated dependencies: JUnit 4.7->4.13.2, cglib-nodep 2.1_3->3.3.0,
  aspectjweaver 1.6.11->1.9.21
- Upgrade Java source/target from 1.6 to 1.8 (required for Java 17 compat)
- Add maven-surefire-plugin 3.2.5 with --add-opens for Java 17 module access
- Replace e.printStackTrace() with proper exception propagation in
  BeanDefinition.setBeanClassName()
- Add input validation for null/empty class names in BeanDefinition
- Fix resource leak: wrap InputStream close in try-finally block
- Applied fixes to both src/ and src+/ copies of affected files

Co-Authored-By: Scoldfield <[email protected]>
@devin-ai-integration
Copy link
Copy Markdown
Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

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.

1 participant