GitHub user adriannistor opened a pull request:
https://github.com/apache/tomcat/pull/123
Checking the signing service response
The code at lines
[398](https://github.com/apache/tomcat/blob/1fde5d696a5d23ed589d05fb6cf6307d5adb563c/java/org/apache/tomcat/buildutil/SignCode.java#L398)
---
[403](https://github.com/apache/tomcat/blob/1fde5d696a5d23ed589d05fb6cf6307d5adb563c/java/org/apache/tomcat/buildutil/SignCode.java#L403)
in file
[SignCode.java](https://github.com/apache/tomcat/blob/trunk/java/org/apache/tomcat/buildutil/SignCode.java)
assumes, without
checking, that the signing service will return the same number of
files and in the same order as in the `List<File>` files parameter ([line
393](https://github.com/apache/tomcat/blob/1fde5d696a5d23ed589d05fb6cf6307d5adb563c/java/org/apache/tomcat/buildutil/SignCode.java#L393)).
If the signing service returns fewer file, then
`zis.getNextEntry()` ([line
400](https://github.com/apache/tomcat/blob/1fde5d696a5d23ed589d05fb6cf6307d5adb563c/java/org/apache/tomcat/buildutil/SignCode.java#L400))
will return `null`
and
`zis.read(buf)` ([line
402](https://github.com/apache/tomcat/blob/1fde5d696a5d23ed589d05fb6cf6307d5adb563c/java/org/apache/tomcat/buildutil/SignCode.java#L402))
will fail (because there is no entry to read from).
Similarly, if the order is different,
`fos.write(buf, 0 , numRead);` ([line
403](https://github.com/apache/tomcat/blob/1fde5d696a5d23ed589d05fb6cf6307d5adb563c/java/org/apache/tomcat/buildutil/SignCode.java#L403))
will overwrite the wrong files.
The above two assumptions about the signing service are fine, but it
is best to have a sanity check for them.
This pull request adds a sanity check for the number of files.
The full check (which also checks for the name equality) is:
```
ZipEntry zisEntry;
if (((zisEntry = zis.getNextEntry()) == null) ||
!zisEntry.getName().equals(files.get(i).getName())) {
throw new BuildException("Signing failed. Malformed service reply.");
}
```
I am not familiar enough with Tomcat code to be sure the
`zisEntry.getName()` and `files.get(i).getName()` calls are correct,
so this pull request does not have name checks (but you can just
copy-paste the above code for the name checks).
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/adriannistor/tomcat checkSigningServiceResult
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/tomcat/pull/123.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #123
----
commit addad0cec94d2535a435de50a6d1bb066fa90b33
Author: Adrian <adrnisto@...>
Date: 2018-10-02T21:07:09Z
Sanity check: the signing service returned the same number of files as the
List<File> files parameter. See pull request for a check on the file names too.
----
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
