[
https://issues.apache.org/jira/browse/OFBIZ-9973?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16904463#comment-16904463
]
Jacques Le Roux commented on OFBIZ-9973:
----------------------------------------
Almost a year ago we received this security report by Man Yue Mo from Semmle:
{quote}
From: "Man Yue Mo via RT" <security-repo...@semmle.com>
To: secur...@ofbiz.apache.org
Subject: [Semmle Security Reports #17190] Path traversal by authenticated users
Date: 2018/09/24 15:05:43
Private: YES
List: secur...@ofbiz.apache.org
Hi,
There are two path traversal issues by authenticated users. To reproduce these
issues from a fresh install, first upload an image via the url
https://localhost:8443/catalog/control/ImageUpload
to ensure that the directory `/framework/images/webapp/products/management`
exists.
1. The first one allows arbitrary file write, which is effectively remote code
execution as the user can write any jsp or ftl files and get them executed by
fetching them again. In the `uploadFrame` method of `FrameImage` class, a file
is created here:
https://github.com/apache/ofbiz/blob/release16.11/applications/product/src/main/java/org/apache/ofbiz/product/imagemanagement/FrameImage.java#L293
The local variable `imageName` used in `imagePath` is taken from the
`imageFileName` component of the map `tempFile`:
https://github.com/apache/ofbiz/blob/release16.11/applications/product/src/main/java/org/apache/ofbiz/product/imagemanagement/FrameImage.java#L264
which comes from the `name` property of an `FileItem`:
https://github.com/apache/ofbiz/blob/release16.11/applications/content/src/main/java/org/apache/ofbiz/content/layout/LayoutWorker.java#L106
This `name` property is taken from the multi-part request as the `filename`
attribute and is in complete control of a remote user.
For example, run the attached `PoC_path.py` will create a `abc.jsp` file in the
root directory of the `ofbiz` repo with `abc` inside it. (`ofbiz/abc.jsp` when
running an instance built from code pulled from github.) This, however, does
not allow overriding of files.
2. In the `previewFrameImage` method of the same class, `productId` is taken
from request query parameter:
https://github.com/apache/ofbiz/blob/release16.11/applications/product/src/main/java/org/apache/ofbiz/product/imagemanagement/FrameImage.java#L340
This is then used to construct a path to fetch an image:
https://github.com/apache/ofbiz/blob/release16.11/applications/product/src/main/java/org/apache/ofbiz/product/imagemanagement/FrameImage.java#L391
I believe this was also noticed here:
https://jira.apache.org/jira/browse/OFBIZ-9973
However, the conclusion of the ticket is incorrect. First, although `imageName`
is encoded, `productId` is not so it is still possible to traverse path.
Second, `getCanonicalFile` has nothing to do with preventing path traversal,
all it does is to resolve the path and get rid of the `../` and `./` in the
path.
This can be tested, for example, by putting an image in the directory
(image.png) containing the ofbiz repository, and use the following url:
https://localhost:8443/catalog/control/previewFrameImage?productId=..%2F..%2F..%2F..%2F..%2F..%2F..%2F&frameContentId=10000&frameDataResourceId=10000&imageWidth=-1&imageHeight=-1&imageName=image.png
Here `frameContentId` and `frameDataResourceId` have to be valid and
corresponds to an image, from some trial and error, it seems like these ids
starts from 10000 and increment when new resource is uploaded, so 10000 has a
good chance of working. If successful, you should see an the target image
image.png overlay with the image corresponding to the one with `frameContentId`
10000. This will only allow arbitrary reads of image files.
As a fresh instalment of ofbiz only comes with the admin user, I don't know how
much privilege is required to access these endpoints or whether this component
is just a sample app.
These are tested on the branch release16.11 (commit 4a0f061). Thank you very
much for your help and please let me know if there is anything that I can help.
Thanks.
Best Regards,
Man Yue Mo
{quote}
As both need an authenticated user they were rejected as not needing a CVE.
Nevertheless, Man Yue Mo gave us interesting information. Following Man Yue Mo
indication I fixed the 1st issue in
Trunk r1864881
R18 r1864882
R17 r1864883
R16 r1864884
Spotbug still reports a security bug in FrameImage.::previewFrameImage. But
still only in trunk, which is weird. Here is the XML content of the report
(found in My Eclipse workspace)
{code:xml}
<BugInstance type="PT_RELATIVE_PATH_TRAVERSAL" priority="2" rank="7"
abbrev="PT" category="SECURITY" first="5">
<Class classname="org.apache.ofbiz.product.imagemanagement.FrameImage">
<SourceLine
classname="org.apache.ofbiz.product.imagemanagement.FrameImage"
sourcefile="FrameImage.java"
sourcepath="org/apache/ofbiz/product/imagemanagement/FrameImage.java"/>
</Class>
<Method classname="org.apache.ofbiz.product.imagemanagement.FrameImage"
name="previewFrameImage"
signature="(Ljavax/servlet/http/HttpServletRequest;Ljavax/servlet/http/HttpServletResponse;)Ljava/lang/String;"
isStatic="true">
<SourceLine
classname="org.apache.ofbiz.product.imagemanagement.FrameImage" start="354"
end="427" startBytecode="0" endBytecode="1457" sourcefile="FrameImage.java"
sourcepath="org/apache/ofbiz/product/imagemanagement/FrameImage.java"/>
</Method>
<Method classname="java.io.File" name="<init>"
signature="(Ljava/lang/String;)V" isStatic="false" role="METHOD_CALLED">
<SourceLine classname="java.io.File" start="275" end="281"
startBytecode="0" endBytecode="115" sourcefile="File.java"
sourcepath="java/io/File.java"/>
</Method>
<String value="imageName" role="STRING_PARAMETER_NAME"/>
<SourceLine classname="org.apache.ofbiz.product.imagemanagement.FrameImage"
start="360" end="360" startBytecode="61" endBytecode="61"
sourcefile="FrameImage.java"
sourcepath="org/apache/ofbiz/product/imagemanagement/FrameImage.java"
role="SOURCE_LINE_GENERATED_AT"/>
<SourceLine classname="org.apache.ofbiz.product.imagemanagement.FrameImage"
start="402" end="402" startBytecode="490" endBytecode="490"
sourcefile="FrameImage.java"
sourcepath="org/apache/ofbiz/product/imagemanagement/FrameImage.java"/>
<SourceLine classname="org.apache.ofbiz.product.imagemanagement.FrameImage"
start="402" end="402" startBytecode="490" endBytecode="490"
sourcefile="FrameImage.java"
sourcepath="org/apache/ofbiz/product/imagemanagement/FrameImage.java"/>
</BugInstance>
<BugInstance type="PT_RELATIVE_PATH_TRAVERSAL" priority="2" rank="7"
abbrev="PT" category="SECURITY" first="1" last="4" removedByChange="true">
<Class classname="org.apache.ofbiz.product.imagemanagement.FrameImage">
<SourceLine
classname="org.apache.ofbiz.product.imagemanagement.FrameImage"
sourcefile="FrameImage.java"
sourcepath="org/apache/ofbiz/product/imagemanagement/FrameImage.java"/>
</Class>
<Method classname="org.apache.ofbiz.product.imagemanagement.FrameImage"
name="previewFrameImage"
signature="(Ljavax/servlet/http/HttpServletRequest;Ljavax/servlet/http/HttpServletResponse;)Ljava/lang/String;"
isStatic="true">
<SourceLine
classname="org.apache.ofbiz.product.imagemanagement.FrameImage" start="355"
end="435" startBytecode="0" endBytecode="1496" sourcefile="FrameImage.java"
sourcepath="org/apache/ofbiz/product/imagemanagement/FrameImage.java"/>
</Method>
<Method classname="java.io.File" name="<init>"
signature="(Ljava/lang/String;)V" isStatic="false" role="METHOD_CALLED">
<SourceLine classname="java.io.File" start="275" end="281"
startBytecode="0" endBytecode="115" sourcefile="File.java"
sourcepath="java/io/File.java"/>
</Method>
<String value="productId" role="STRING_PARAMETER_NAME"/>
<SourceLine classname="org.apache.ofbiz.product.imagemanagement.FrameImage"
start="360" end="360" startBytecode="48" endBytecode="48"
sourcefile="FrameImage.java"
sourcepath="org/apache/ofbiz/product/imagemanagement/FrameImage.java"
role="SOURCE_LINE_GENERATED_AT"/>
<SourceLine classname="org.apache.ofbiz.product.imagemanagement.FrameImage"
start="410" end="410" startBytecode="512" endBytecode="512"
sourcefile="FrameImage.java"
sourcepath="org/apache/ofbiz/product/imagemanagement/FrameImage.java"/>
<SourceLine classname="org.apache.ofbiz.product.imagemanagement.FrameImage"
start="410" end="410" startBytecode="512" endBytecode="512"
sourcefile="FrameImage.java"
sourcepath="org/apache/ofbiz/product/imagemanagement/FrameImage.java"/>
</BugInstance>
{code}
Obviously it does not take into account the use of normalize(). I have checked
and both issues are fixed by using
[Path::normalize|https://docs.oracle.com/javase/8/docs/api/java/nio/file/Path.html#normalize--]
as adviced by [OWASP in another
way|https://www.owasp.org/index.php/File_System#Path_traversal]:
bq. If forced to use user input for file operations, normalize the input before
using in file io API's, such as
http://docs.oracle.com/javase/7/docs/api/java/net/URI.html#normalize().
> [FB] Find Security Bugs
> -----------------------
>
> Key: OFBIZ-9973
> URL: https://issues.apache.org/jira/browse/OFBIZ-9973
> Project: OFBiz
> Issue Type: Sub-task
> Components: marketing, product
> Affects Versions: Trunk, Release Branch 16.11
> Reporter: Jacques Le Roux
> Assignee: Jacques Le Roux
> Priority: Major
> Fix For: 17.12.01, 16.11.06, 18.12.01
>
>
> I recently
> [found|https://www.ysofters.com/2015/08/31/taint-analysis-added-to-findbugs/]
> FindBugs embeds an option [to Find Security
> Bugs|https://find-sec-bugs.github.io/]:
> I have tried this option:
> https://github.com/find-sec-bugs/find-sec-bugs/wiki/Eclipse-Tutorial
> Also later we should remember of OFBIZ-7963 and if possible run this tool in
> [Builbot using
> Gradle|https://search.maven.org/#search|gav|1|g:%22com.h3xstream.findsecbugs%22%20AND%20a:%22findsecbugs-plugin%22]
> (did not check feasibility)
--
This message was sent by Atlassian JIRA
(v7.6.14#76016)
