ofbiz-notifications mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Jacques Le Roux (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (OFBIZ-9973) [FB] Find Security Bugs
Date Sat, 10 Aug 2019 15:59:00 GMT

    [ 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-reports@semmle.com>
To: security@ofbiz.apache.org
Subject: [Semmle Security Reports #17190] Path traversal by authenticated users
Date: 2018/09/24 15:05:43
Private: YES
List: security@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="&lt;init&gt;" 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="&lt;init&gt;" 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)

Mime
View raw message