Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Suppress LGTM alert for ZipSlip in PluginMonitor #2145

Merged
merged 1 commit into from
Nov 10, 2022

Conversation

guusdk
Copy link
Member

@guusdk guusdk commented Nov 10, 2022

ZipSlip is possible only when explicitly enabled by an administrator. No need to have a static analyzer alert for this particular case.

ZipSlip is possible only when explicitly enabled by an administrator. No need to have a static analyzer alert for this particular case.
@guusdk guusdk force-pushed the lgtm-suppress-zipslip branch from 63463c9 to 4fd11d3 Compare November 10, 2022 09:04
@@ -479,7 +479,9 @@
for ( Enumeration e = zipFile.entries(); e.hasMoreElements(); )
{
JarEntry entry = (JarEntry) e.nextElement();
Path entryFile = dir.resolve( entry.getName() ); // ignore possibility for zipslip as this is sanitized for if property is enabled lgtm [java/zipslip]
Path entryFile = dir.resolve( entry.getName() ); /* lgtm[java/zipslip] */

Check failure

Code scanning / CodeQL

Arbitrary file write during archive extraction ("Zip Slip")

Unsanitized archive entry, which may contain '..', is used in a [file system operation](1).
@guusdk
Copy link
Member Author

guusdk commented Nov 10, 2022

An additional observation: if a malicious attacker is able to upload a plugin (that might have a ZipSlip vulnerability), they are able to upload executable code. I wonder to what extend guarding against ZipSlip here actually improve security significantly.

@guusdk guusdk merged commit d354fa7 into igniterealtime:main Nov 10, 2022
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