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

Fixing scanning issue of jars inside war files #22

Merged
merged 3 commits into from
Dec 17, 2021

Conversation

dariux
Copy link
Contributor

@dariux dariux commented Dec 17, 2021

Current version would not scan zip files inside zip files properly (i.e., log4j jars inside *.war files)

@yunzheng
Copy link
Member

Do you have an example war file where it goes wrong?

I tested on the following WAR file: https://get.jenkins.io/war-stable/1.409.3/jenkins.war

python3 log4j-finder.py /tmp/war -q
[2021-12-17 00:13:52] UNKNOWN: /private/tmp/war/jenkins.war -> winstone.jar -> winstone/JNDIManager.class [fc0814d1ed17c42eee4ea62ad8646f2b: Unknown MD5]

@dariux
Copy link
Contributor Author

dariux commented Dec 17, 2021

You're right, it works on Python 3.8.5. On Python 3.6.9, the same command gives this (i.e., fails silently):

python3 log4j-finder.py jenkins.war --no-banner
[2021-12-17 01:41:05] MediaPC2 Scanning: jenkins.war
[2021-12-17 01:41:05] MediaPC2 Finished scan, elapsed time: 0.02 seconds

Summary:
 Processed 0 files and 0 directories
 Scanned 1 files

Elapsed time: 0.02 seconds

With the debug output:
2021-12-16 20:43:18,320 DEBUG <zipfile.ZipExtFile name='winstone.jar' mode='r' compress_type=deflate>: File is not a zip file

@dariux
Copy link
Contributor Author

dariux commented Dec 17, 2021

Just realized Python 3.6.9 does not work anyway with more recent changes

Traceback (most recent call last):
  File "log4j-finder.py", line 329, in <module>
    sys.exit(main())
  File "log4j-finder.py", line 304, in main
    has_lookup = zipfile.Path(zfile, lookup_path).exists()
AttributeError: module 'zipfile' has no attribute 'Path'

Until zipfile.Path was added, this would have fixed the script to work with Python < 3.8

@dariux
Copy link
Contributor Author

dariux commented Dec 17, 2021

I'll close for now.

@dariux dariux closed this Dec 17, 2021
@yunzheng
Copy link
Member

@dariux Hi dariux, i fixed the zipfile.Patherror, and it should be Python 3.6 compatible again. Feel free to open the PR again if it still doesn't work for you.

@dariux dariux reopened this Dec 17, 2021
@dariux
Copy link
Contributor Author

dariux commented Dec 17, 2021

Thanks, should work now. Python 3.8.5 - same output (still works).

Python 3.6.9 BEFORE:

python3 log4j-finder.py /tmp/jenkins.war --no-banner
[2021-12-17 11:24:50] PC Scanning: /tmp/jenkins.war
[2021-12-17 11:24:50] PC Finished scan, elapsed time: 0.02 seconds

Summary:
 Processed 0 files and 0 directories
 Scanned 1 files

Python 3.6.9 AFTER:

python3 log4j-finder.py /tmp/jenkins.war --no-banner
[2021-12-17 11:25:43] PC Scanning: /tmp/jenkins.war
[2021-12-17 11:25:43] PC UNKNOWN: /tmp/jenkins.war -> winstone.jar -> winstone/JNDIManager.class [fc0814d1ed17c42eee4ea62ad8646f2b: Unknown MD5]
[2021-12-17 11:25:43] MediaPC2 Finished scan, elapsed time: 0.46 seconds

Summary:
 Processed 0 files and 0 directories
 Scanned 1 files
  Found 1 unknown files

@yunzheng
Copy link
Member

Before I merge I want to check what causes this issue. Now it loads the zip file in memory instead of a file handle, which can be problematic if it's a big file.

@yunzheng
Copy link
Member

Made a small test case:

import zipfile

# https://get.jenkins.io/war-stable/1.409.3/jenkins.war
zfile = zipfile.ZipFile("jenkins.war")
zinfo = zfile.infolist()[1332]
print(zinfo)
zf = zfile.open(zinfo.filename)
print(zf)
test = zipfile.ZipFile(zf)
print(test)
print(len(test.infolist()))

Dockerfile for easy testing:

ARG  VERSION
FROM python:${VERSION}

RuN echo "building version ${VERSION}"
COPY jenkins.war /
COPY test.py /

CMD python3 test.py
$ docker build -t zipfile:3.6.9 --build-arg VERSION=3.6.9 .
$ docker build -t zipfile:3.6.10 --build-arg VERSION=3.6.10 .
$ docker build -t zipfile:3.7.0 --build-arg VERSION=3.7.0 .
$ docker build -t zipfile:3.7.2 --build-arg VERSION=3.7.2 .

# run using:
$ docker run -it --rm zipfile:3.6.9    # breaks
$ docker run -it --rm zipfile:3.6.10  # breaks
$ docker run -it --rm zipfile:3.7.0    # breaks
$ docker run -it --rm zipfile:3.7.2    # ok

@yunzheng
Copy link
Member

@yunzheng yunzheng merged commit abece01 into fox-it:main Dec 17, 2021
@dariux
Copy link
Contributor Author

dariux commented Dec 17, 2021

Yes, you were too quick for me to respond, those are the correct references. The downside is it loads individual jars into memory; the upside - does not fail scanning war files silently. Thanks!
Ideally, maybe we could check for the correct Python version via try/except; it does seem like it might slow down the scan considerably, which is avoidable for versions >= 3.7.2

yunzheng added a commit to yunzheng/log4j-finder that referenced this pull request Dec 17, 2021
@yunzheng
Copy link
Member

Ideally, maybe we could check for the correct Python version via try/except; it does seem like it might slow down the scan considerably, which is avoidable for versions >= 3.7.2

Yes agreed, i have modified your patch to do just this, See #33
Best of both worlds! :)

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.

None yet

2 participants