Browse Source

BasePackConnection: Check for expected length of ref advertisement

When a server sends a ref advertisement using protocol v2 it contains
lines other than ref names and sha1s.  Attempting to get the sha1 out
of such a line using the substring method can result in a SIOOB error
when it doesn't actually contain the sha1 and ref name.

Add a check that the line is of the expected length, and subsequently
that the extracted object id is valid, and if not throw an exception.

Change-Id: Id92fe66ff8b6deb2cf987d81929f8d0602c399f4
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
stable-5.1
David Pursehouse 6 years ago
parent
commit
f4fc6404ba
  1. 1
      org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties
  2. 1
      org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java
  3. 16
      org.eclipse.jgit/src/org/eclipse/jgit/transport/BasePackConnection.java

1
org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties

@ -390,6 +390,7 @@ invalidPathPeriodAtEndWindows=Invalid path (period at end is ignored by Windows)
invalidPathSpaceAtEndWindows=Invalid path (space at end is ignored by Windows): {0} invalidPathSpaceAtEndWindows=Invalid path (space at end is ignored by Windows): {0}
invalidPathReservedOnWindows=Invalid path (''{0}'' is reserved on Windows): {1} invalidPathReservedOnWindows=Invalid path (''{0}'' is reserved on Windows): {1}
invalidRedirectLocation=Invalid redirect location {0} -> {1} invalidRedirectLocation=Invalid redirect location {0} -> {1}
invalidRefAdvertisementLine=Invalid ref advertisement line: ''{1}''
invalidReflogRevision=Invalid reflog revision: {0} invalidReflogRevision=Invalid reflog revision: {0}
invalidRefName=Invalid ref name: {0} invalidRefName=Invalid ref name: {0}
invalidReftableBlock=Invalid reftable block invalidReftableBlock=Invalid reftable block

1
org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java

@ -451,6 +451,7 @@ public class JGitText extends TranslationBundle {
/***/ public String invalidPathSpaceAtEndWindows; /***/ public String invalidPathSpaceAtEndWindows;
/***/ public String invalidPathReservedOnWindows; /***/ public String invalidPathReservedOnWindows;
/***/ public String invalidRedirectLocation; /***/ public String invalidRedirectLocation;
/***/ public String invalidRefAdvertisementLine;
/***/ public String invalidReflogRevision; /***/ public String invalidReflogRevision;
/***/ public String invalidRefName; /***/ public String invalidRefName;
/***/ public String invalidReftableBlock; /***/ public String invalidReftableBlock;

16
org.eclipse.jgit/src/org/eclipse/jgit/transport/BasePackConnection.java

@ -57,6 +57,7 @@ import java.util.HashSet;
import java.util.LinkedHashMap; import java.util.LinkedHashMap;
import java.util.Set; import java.util.Set;
import org.eclipse.jgit.errors.InvalidObjectIdException;
import org.eclipse.jgit.errors.NoRemoteRepositoryException; import org.eclipse.jgit.errors.NoRemoteRepositoryException;
import org.eclipse.jgit.errors.PackProtocolException; import org.eclipse.jgit.errors.PackProtocolException;
import org.eclipse.jgit.errors.RemoteRepositoryException; import org.eclipse.jgit.errors.RemoteRepositoryException;
@ -222,6 +223,10 @@ abstract class BasePackConnection extends BaseConnection {
} }
} }
// Expecting to get a line in the form "sha1 refname"
if (line.length() < 41 || line.charAt(40) != ' ') {
throw invalidRefAdvertisementLine(line);
}
String name = line.substring(41, line.length()); String name = line.substring(41, line.length());
if (avail.isEmpty() && name.equals("capabilities^{}")) { //$NON-NLS-1$ if (avail.isEmpty() && name.equals("capabilities^{}")) { //$NON-NLS-1$
// special line from git-receive-pack to show // special line from git-receive-pack to show
@ -229,7 +234,12 @@ abstract class BasePackConnection extends BaseConnection {
continue; continue;
} }
final ObjectId id = ObjectId.fromString(line.substring(0, 40)); final ObjectId id;
try {
id = ObjectId.fromString(line.substring(0, 40));
} catch (InvalidObjectIdException e) {
throw invalidRefAdvertisementLine(line);
}
if (name.equals(".have")) { //$NON-NLS-1$ if (name.equals(".have")) { //$NON-NLS-1$
additionalHaves.add(id); additionalHaves.add(id);
} else if (name.endsWith("^{}")) { //$NON-NLS-1$ } else if (name.endsWith("^{}")) { //$NON-NLS-1$
@ -318,6 +328,10 @@ abstract class BasePackConnection extends BaseConnection {
return new PackProtocolException(uri, MessageFormat.format(JGitText.get().duplicateAdvertisementsOf, name)); return new PackProtocolException(uri, MessageFormat.format(JGitText.get().duplicateAdvertisementsOf, name));
} }
private PackProtocolException invalidRefAdvertisementLine(String line) {
return new PackProtocolException(uri, MessageFormat.format(JGitText.get().invalidRefAdvertisementLine, line));
}
/** {@inheritDoc} */ /** {@inheritDoc} */
@Override @Override
public void close() { public void close() {

Loading…
Cancel
Save