[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: upload ant



Hi Roberto.

On Wednesday 18 July 2018 05:46 PM, Roberto C. Sánchez wrote:
> Hi Abhijith,
> 
> A few notes:
> 
> - Your debian/changelog entry has trailing whitespace, which should be
>   removed

Done

> - You additionally need this commit for a minor documentation/typo fix:
>   https://github.com/apache/ant/commit/19910e518a669c8cc4d9b74c9ab11471c18cb634

Done

> - In the documentation changes you modified upstream's "since 1.9.12" to
>   "since 1.9.4-3+deb8u1", which is good, but you left it as 1.9.12
>   in once place; all instances should be changed, I think (note that the
>   additional commit I mentioned above contains another instance of the
>   version number that needs to be changed)

Done

> - You mention running the testEntriesDontEscapeDestByDefault and
>   testEntriesCanEscapeDestIfRequested tests, but do not mention the
>   testEntriesCanEscapeDestViaAbsolutePathByDefault and
>   testEntriesDontEscapeDestViaAbsolutePathIfProhibited tests; can you
>   run the other two and confirm that they also pass?

The test names are little different. I followed tests from master[1]
branch. But yes, I ran stripping absolute path tests successfully.

> 
> If you can make these corrections and confirm the additional tests, your
> changes will be ready to upload.
> 

Made all the corrections. Thanks for the review.


--abhijith

diff -Nru ant-1.9.4/debian/changelog ant-1.9.4/debian/changelog
--- ant-1.9.4/debian/changelog	2014-10-08 01:08:52.000000000 +0200
+++ ant-1.9.4/debian/changelog	2018-07-18 13:03:03.000000000 +0200
@@ -1,3 +1,13 @@
+ant (1.9.4-3+deb8u1) jessie-security; urgency=high
+
+  * Non-maintainer upload by the Debian LTS Team
+  * Fix CVE-2018-10886: unzip and untar targets allow the extraction of
+    files outside the target directory. A crafted zip or tar file
+    submitted to an Ant build could create or overwrite arbitrary files
+    with the privileges of the user running Ant
+
+ -- Abhijith PA <abhijith@disroot.org>  Wed, 18 Jul 2018 16:33:03 +0530
+
 ant (1.9.4-3) unstable; urgency=medium
 
   * Removed the patch adding Xerces to the Ant classpath
diff -Nru ant-1.9.4/debian/patches/CVE-2018-10886.patch ant-1.9.4/debian/patches/CVE-2018-10886.patch
--- ant-1.9.4/debian/patches/CVE-2018-10886.patch	1970-01-01 01:00:00.000000000 +0100
+++ ant-1.9.4/debian/patches/CVE-2018-10886.patch	2018-07-18 13:03:03.000000000 +0200
@@ -0,0 +1,118 @@
+Description: CVE-2018-10886
+ unzip and untar targets allows the extraction of files outside the target 
+ directory. A crafted zip or tar file submitted to an Ant build could create or 
+ overwrite arbitrary files with the privileges of the user running Ant.
+ 
+Author: Abhijith PA <abhijith@disroot.org>
+Origin: https://github.com/apache/ant/commit/e56e54565804991c62ec76dad385d2bdda8972a7
+        https://github.com/apache/ant/commit/1a2b1e37e3616991588f21efa89c474dd6ff83ff
+        https://github.com/apache/ant/commit/f72406d53cfb3b3425cc9d000eea421a0e05d8fe
+        https://github.com/apache/ant/commit/857095da5153fd18504b46f276d84f1e76a66970
+Last-Update: 2018-07-18
+
+Index: ant-1.9.4/manual/Tasks/unzip.html
+===================================================================
+--- ant-1.9.4.orig/manual/Tasks/unzip.html
++++ ant-1.9.4/manual/Tasks/unzip.html
+@@ -125,8 +125,9 @@ archive.</p>
+       Note that this changes the entry's name before applying
+       include/exclude patterns and before using the nested mappers (if
+       any).  <em>since Ant 1.8.0</em></td>
+-    <td valign="top" align="center">No, defaults to false</td>
+-  </tr>
++    <td valign="top" align="center">No, defaults to true <em>since 1.9.4-3+deb8u1</em>
++      (used to default to false prior to that)</td>
++</tr>
+   <tr>
+     <td valign="top">scanForUnicodeExtraFields</td>
+     <td valign="top"><b>Note:</b> This attribute is not available for
+@@ -137,6 +138,15 @@ archive.</p>
+       zip task page</a></td>
+     <td align="center" valign="top">No, defaults to true</td>
+   </tr>
++  <tr>
++    <td valign="top">allowFilesToEscapeDest</td>
++    <td valign="top">Whether to allow the extracted file or directory
++      to be outside of the dest directory.
++      <em>since Ant 1.9.4-3+deb8u1</em></td>
++    <td valign="top" align="center">No, defaults to false unless
++    stripAbsolutePathSpec is true and the entry's name starts with a leading
++    path spec.</td>
++  </tr>
+ </table>
+ <h3>Examples</h3>
+ <pre>
+Index: ant-1.9.4/src/main/org/apache/tools/ant/taskdefs/Expand.java
+===================================================================
+--- ant-1.9.4.orig/src/main/org/apache/tools/ant/taskdefs/Expand.java
++++ ant-1.9.4/src/main/org/apache/tools/ant/taskdefs/Expand.java
+@@ -67,8 +67,9 @@ public class Expand extends Task {
+     private Union resources = new Union();
+     private boolean resourcesSpecified = false;
+     private boolean failOnEmptyArchive = false;
+-    private boolean stripAbsolutePathSpec = false;
++    private boolean stripAbsolutePathSpec = true;
+     private boolean scanForUnicodeExtraFields = true;
++    private Boolean allowFilesToEscapeDest = null;
+ 
+     public static final String NATIVE_ENCODING = "native-encoding";
+ 
+@@ -240,14 +241,17 @@ public class Expand extends Task {
+                                boolean isDirectory, FileNameMapper mapper)
+                                throws IOException {
+ 
+-        if (stripAbsolutePathSpec && entryName.length() > 0
++        final boolean entryNameStartsWithPathSpec = entryName.length() > 0
+             && (entryName.charAt(0) == File.separatorChar
+                 || entryName.charAt(0) == '/'
+-                || entryName.charAt(0) == '\\')) {
++                || entryName.charAt(0) == '\\');
++        if (stripAbsolutePathSpec && entryNameStartsWithPathSpec) {
+             log("stripped absolute path spec from " + entryName,
+                 Project.MSG_VERBOSE);
+             entryName = entryName.substring(1);
+         }
++        boolean allowedOutsideOfDest = Boolean.TRUE == getAllowFilesToEscapeDest()
++            || null == getAllowFilesToEscapeDest() && !stripAbsolutePathSpec && entryNameStartsWithPathSpec;
+ 
+         if (patternsets != null && patternsets.size() > 0) {
+             String name = entryName.replace('/', File.separatorChar)
+@@ -313,6 +317,12 @@ public class Expand extends Task {
+             mappedNames = new String[] {entryName};
+         }
+         File f = fileUtils.resolveFile(dir, mappedNames[0]);
++        if (!allowedOutsideOfDest && !fileUtils.isLeadingPath(dir, f)) {
++            log("skipping " + entryName + " as its target " + f + " is outside of "
++                + dir + ".", Project.MSG_VERBOSE);
++                return;
++        }
++
+         try {
+             if (!overwrite && f.exists()
+                 && f.lastModified() >= entryDate.getTime()) {
+@@ -508,4 +518,25 @@ public class Expand extends Task {
+         return scanForUnicodeExtraFields;
+     }
+ 
++    /**
++     * Whether to allow the extracted file or directory to be outside of the dest directory.
++     *
++     * @param b the flag
++     * @since Ant 1.9.4-3+deb8u1
++     */
++    public void setAllowFilesToEscapeDest(boolean b) {
++        allowFilesToEscapeDest = b;
++    }
++
++    /**
++     * Whether to allow the extracted file or directory to be outside of the dest directory.
++     *
++     * @return {@code null} if the flag hasn't been set explicitly,
++     * otherwise the value set by the user.
++     * @since Ant 1.9.4-3+deb8u1
++     */
++    public Boolean getAllowFilesToEscapeDest() {
++        return allowFilesToEscapeDest;
++    }
++
+ }
diff -Nru ant-1.9.4/debian/patches/series ant-1.9.4/debian/patches/series
--- ant-1.9.4/debian/patches/series	2014-10-07 23:35:43.000000000 +0200
+++ ant-1.9.4/debian/patches/series	2018-07-18 13:03:03.000000000 +0200
@@ -3,3 +3,4 @@
 0006-fix-ANT_HOME-path.patch
 0007-use-build.classpath.patch
 0008-junit4-replace-assumeFalse.patch
+CVE-2018-10886.patch

Reply to: