diff options
5 files changed, 155 insertions, 25 deletions
diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/PushConnectionTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/PushConnectionTest.java index 33a910505f..4aebc1d32c 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/PushConnectionTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/PushConnectionTest.java @@ -173,4 +173,27 @@ public class PushConnectionTest { } } } + + @Test + public void limitCommandBytes() throws IOException { + Map<String, RemoteRefUpdate> updates = new HashMap<>(); + for (int i = 0; i < 4; i++) { + RemoteRefUpdate rru = new RemoteRefUpdate( + null, null, obj2, "refs/test/T" + i, + false, null, ObjectId.zeroId()); + updates.put(rru.getRemoteName(), rru); + } + + server.getConfig().setInt("receive", null, "maxCommandBytes", 170); + try (Transport tn = testProtocol.open(uri, client, "server"); + PushConnection connection = tn.openPush()) { + try { + connection.push(NullProgressMonitor.INSTANCE, updates); + fail("server did not abort"); + } catch (TransportException e) { + String msg = e.getMessage(); + assertEquals("remote: Too many commands", msg); + } + } + } } diff --git a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties index 8bf2155d93..76d35b0a3b 100644 --- a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties +++ b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties @@ -609,6 +609,7 @@ tagOnRepoWithoutHEADCurrentlyNotSupported=Tag on repository without HEAD current theFactoryMustNotBeNull=The factory must not be null timeIsUncertain=Time is uncertain timerAlreadyTerminated=Timer already terminated +tooManyCommands=Too many commands tooManyIncludeRecursions=Too many recursions; circular includes in config file(s)? topologicalSortRequired=Topological sort required. transactionAborted=transaction aborted diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java index 244356c13b..37499bbb3e 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java @@ -669,6 +669,7 @@ public class JGitText extends TranslationBundle { /***/ public String theFactoryMustNotBeNull; /***/ public String timeIsUncertain; /***/ public String timerAlreadyTerminated; + /***/ public String tooManyCommands; /***/ public String tooManyIncludeRecursions; /***/ public String topologicalSortRequired; /***/ public String transportExceptionBadRef; diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/BaseReceivePack.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/BaseReceivePack.java index 4d0803a339..b20a06864b 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/BaseReceivePack.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/BaseReceivePack.java @@ -97,6 +97,7 @@ import org.eclipse.jgit.revwalk.RevObject; import org.eclipse.jgit.revwalk.RevSort; import org.eclipse.jgit.revwalk.RevTree; import org.eclipse.jgit.revwalk.RevWalk; +import org.eclipse.jgit.transport.PacketLineIn.InputOverLimitIOException; import org.eclipse.jgit.transport.ReceiveCommand.Result; import org.eclipse.jgit.util.io.InterruptTimer; import org.eclipse.jgit.util.io.LimitedInputStream; @@ -244,6 +245,8 @@ public abstract class BaseReceivePack { String userAgent; private Set<ObjectId> clientShallowCommits; private List<ReceiveCommand> commands; + private long maxCommandBytes; + private long maxDiscardBytes; private StringBuilder advertiseError; @@ -318,6 +321,8 @@ public abstract class BaseReceivePack { allowNonFastForwards = rc.allowNonFastForwards; allowOfsDelta = rc.allowOfsDelta; allowPushOptions = rc.allowPushOptions; + maxCommandBytes = rc.maxCommandBytes; + maxDiscardBytes = rc.maxDiscardBytes; advertiseRefsHook = AdvertiseRefsHook.DEFAULT; refFilter = RefFilter.DEFAULT; advertisedHaves = new HashSet<ObjectId>(); @@ -338,7 +343,8 @@ public abstract class BaseReceivePack { final boolean allowNonFastForwards; final boolean allowOfsDelta; final boolean allowPushOptions; - + final long maxCommandBytes; + final long maxDiscardBytes; final SignedPushConfig signedPush; ReceiveConfig(final Config config) { @@ -350,6 +356,12 @@ public abstract class BaseReceivePack { true); allowPushOptions = config.getBoolean("receive", "pushoptions", //$NON-NLS-1$ //$NON-NLS-2$ false); + maxCommandBytes = config.getLong("receive", //$NON-NLS-1$ + "maxCommandBytes", //$NON-NLS-1$ + 3 << 20); + maxDiscardBytes = config.getLong("receive", //$NON-NLS-1$ + "maxCommandDiscardBytes", //$NON-NLS-1$ + -1); signedPush = SignedPushConfig.KEY.parse(config); } } @@ -729,6 +741,38 @@ public abstract class BaseReceivePack { } /** + * Set the maximum number of command bytes to read from the client. + * + * @param limit + * command limit in bytes; if 0 there is no limit. + * @since 4.7 + */ + public void setMaxCommandBytes(long limit) { + maxCommandBytes = limit; + } + + /** + * Set the maximum number of command bytes to discard from the client. + * <p> + * Discarding remaining bytes allows this instance to consume the rest of + * the command block and send a human readable over-limit error via the + * side-band channel. If the client sends an excessive number of bytes this + * limit kicks in and the instance disconnects, resulting in a non-specific + * 'pipe closed', 'end of stream', or similar generic error at the client. + * <p> + * When the limit is set to {@code -1} the implementation will default to + * the larger of {@code 3 * maxCommandBytes} or {@code 3 MiB}. + * + * @param limit + * discard limit in bytes; if 0 there is no limit; if -1 the + * implementation tries to set a reasonable default. + * @since 4.7 + */ + public void setMaxCommandDiscardBytes(long limit) { + maxDiscardBytes = limit; + } + + /** * Set the maximum allowed Git object size. * <p> * If an object is larger than the given size the pack-parsing will throw an @@ -741,7 +785,6 @@ public abstract class BaseReceivePack { maxObjectSizeLimit = limit; } - /** * Set the maximum allowed pack size. * <p> @@ -1134,13 +1177,16 @@ public abstract class BaseReceivePack { * @throws IOException */ protected void recvCommands() throws IOException { + PacketLineIn pck = maxCommandBytes > 0 + ? new PacketLineIn(rawIn, maxCommandBytes) + : pckIn; PushCertificateParser certParser = getPushCertificateParser(); boolean firstPkt = true; try { for (;;) { String line; try { - line = pckIn.readString(); + line = pck.readString(); } catch (EOFException eof) { if (commands.isEmpty()) return; @@ -1163,13 +1209,13 @@ public abstract class BaseReceivePack { enableCapabilities(); if (line.equals(GitProtocolConstants.OPTION_PUSH_CERT)) { - certParser.receiveHeader(pckIn, !isBiDirectionalPipe()); + certParser.receiveHeader(pck, !isBiDirectionalPipe()); continue; } } if (line.equals(PushCertificateParser.BEGIN_SIGNATURE)) { - certParser.receiveSignature(pckIn); + certParser.receiveSignature(pck); continue; } @@ -1186,18 +1232,31 @@ public abstract class BaseReceivePack { } pushCert = certParser.build(); if (hasCommands()) { - readPostCommands(pckIn); + readPostCommands(pck); } } catch (PackProtocolException e) { - if (sideBand) { - try { - pckIn.discardUntilEnd(); - } catch (IOException e2) { - // Ignore read failures attempting to discard. - } - } + discardCommands(); fatalError(e.getMessage()); throw e; + } catch (InputOverLimitIOException e) { + String msg = JGitText.get().tooManyCommands; + discardCommands(); + fatalError(msg); + throw new PackProtocolException(msg); + } + } + + private void discardCommands() { + if (sideBand) { + long max = maxDiscardBytes; + if (max < 0) { + max = Math.max(3 * maxCommandBytes, 3L << 20); + } + try { + new PacketLineIn(rawIn, max).discardUntilEnd(); + } catch (IOException e) { + // Ignore read failures attempting to discard. + } } } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/PacketLineIn.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/PacketLineIn.java index e142babd68..e70925693b 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/PacketLineIn.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/PacketLineIn.java @@ -87,19 +87,32 @@ public class PacketLineIn { ACK_READY; } + private final byte[] lineBuffer = new byte[SideBandOutputStream.SMALL_BUF]; private final InputStream in; + private long limit; - private final byte[] lineBuffer; + /** + * Create a new packet line reader. + * + * @param in + * the input stream to consume. + */ + public PacketLineIn(InputStream in) { + this(in, 0); + } /** * Create a new packet line reader. * - * @param i + * @param in * the input stream to consume. + * @param limit + * bytes to read from the input; unlimited if set to 0. + * @since 4.7 */ - public PacketLineIn(final InputStream i) { - in = i; - lineBuffer = new byte[SideBandOutputStream.SMALL_BUF]; + public PacketLineIn(InputStream in, long limit) { + this.in = in; + this.limit = limit; } AckNackResult readACK(final MutableObjectId returnedId) throws IOException { @@ -210,15 +223,48 @@ public class PacketLineIn { int readLength() throws IOException { IO.readFully(in, lineBuffer, 0, 4); + int len; try { - final int len = RawParseUtils.parseHexInt16(lineBuffer, 0); - if (len != 0 && len < 4) - throw new ArrayIndexOutOfBoundsException(); - return len; + len = RawParseUtils.parseHexInt16(lineBuffer, 0); } catch (ArrayIndexOutOfBoundsException err) { - throw new IOException(MessageFormat.format(JGitText.get().invalidPacketLineHeader, - "" + (char) lineBuffer[0] + (char) lineBuffer[1] //$NON-NLS-1$ - + (char) lineBuffer[2] + (char) lineBuffer[3])); + throw invalidHeader(); } + + if (len == 0) { + return 0; + } else if (len < 4) { + throw invalidHeader(); + } + + if (limit != 0) { + int n = len - 4; + if (limit < n) { + limit = -1; + try { + IO.skipFully(in, n); + } catch (IOException e) { + // Ignore failure discarding packet over limit. + } + throw new InputOverLimitIOException(); + } + // if set limit must not be 0 (means unlimited). + limit = n < limit ? limit - n : -1; + } + return len; + } + + private IOException invalidHeader() { + return new IOException(MessageFormat.format(JGitText.get().invalidPacketLineHeader, + "" + (char) lineBuffer[0] + (char) lineBuffer[1] //$NON-NLS-1$ + + (char) lineBuffer[2] + (char) lineBuffer[3])); + } + + /** + * IOException thrown by read when the configured input limit is exceeded. + * + * @since 4.7 + */ + public static class InputOverLimitIOException extends IOException { + private static final long serialVersionUID = 1L; } } |