diff --git a/git-proxy-java-core/src/main/java/org/finos/gitproxy/git/GitReceivePackParser.java b/git-proxy-java-core/src/main/java/org/finos/gitproxy/git/GitReceivePackParser.java index 426f69a6..7257691e 100644 --- a/git-proxy-java-core/src/main/java/org/finos/gitproxy/git/GitReceivePackParser.java +++ b/git-proxy-java-core/src/main/java/org/finos/gitproxy/git/GitReceivePackParser.java @@ -2,9 +2,9 @@ import static org.finos.gitproxy.git.GitClientUtils.ZERO_OID; +import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; import java.io.IOException; -import java.nio.charset.StandardCharsets; import java.time.Instant; import java.util.zip.DataFormatException; import java.util.zip.Inflater; @@ -12,6 +12,7 @@ import lombok.Data; import lombok.extern.slf4j.Slf4j; import org.eclipse.jgit.lib.PersonIdent; +import org.eclipse.jgit.transport.PacketLineIn; import org.eclipse.jgit.util.RawParseUtils; @Slf4j @@ -74,18 +75,14 @@ public static PushInfo parsePush(String packetLine, byte[] packData) throws IOEx } public static Commit parsePackData(byte[] data) throws IOException { - int pos = findPackSignature(data); + int pos = findPackDataOffset(data); // Check if pack signature was found if (pos == -1) { throw new IOException("No PACK signature found in data"); } - // Read pack header - byte[] header = new byte[12]; - System.arraycopy(data, pos, header, 0, 12); - - // Skip version and entry count validation + // Skip pack header (signature + version + entry count) pos += 12; // Only parse the first commit object @@ -97,31 +94,38 @@ public static Commit parsePackData(byte[] data) throws IOException { } /** - * Find the start of PACK data. Checks offset 0 first (normal case after ParseGitRequestFilter consumes the pkt-line - * section), then falls back to walking any remaining pkt-lines to find the flush boundary. + * Find the start of PACK data in a git receive-pack request body. Accepts either a body that already starts with + * the PACK signature (the normal case after {@code ParseGitRequestFilter} has consumed the pkt-line section) or a + * full receive-pack body containing pkt-line command frames followed by a flush (0000) and then PACK data. + * + *
Uses JGit's {@link PacketLineIn} to walk the pkt-line framing, which also protects against CVE-2025-54584 (the + * prior byte-scan for 'P','A','C','K' could be spoofed by a crafted ref name such as {@code refs/heads/PACK-evil}). * - *
CVE-2025-54584: replaces the former naive byte-scan for 'P','A','C','K' which could be spoofed by a crafted - * ref name or commit content containing those bytes. + * @return byte offset of the PACK signature, or -1 if none is found */ - private static int findPackSignature(byte[] data) { + public static int findPackDataOffset(byte[] data) { + if (data == null || data.length < 4) { + return -1; + } // Fast path: PACK signature at the start (normal after flush was consumed upstream) - if (data.length >= 4 && data[0] == 'P' && data[1] == 'A' && data[2] == 'C' && data[3] == 'K') { + if (data[0] == 'P' && data[1] == 'A' && data[2] == 'C' && data[3] == 'K') { return 0; } - // Fallback: walk past any remaining pkt-lines to find the flush boundary - int pos = 0; - while (pos + 4 <= data.length) { - int len = parsePacketLength(data, pos); - if (len < 0) break; - if (len == 0) { - pos += 4; - break; + // Walk pkt-line framing via JGit until we hit the flush packet + ByteArrayInputStream bais = new ByteArrayInputStream(data); + PacketLineIn pli = new PacketLineIn(bais); + try { + while (true) { + String line = pli.readString(); + if (PacketLineIn.isEnd(line)) { + break; + } } - if (len < 4 || pos + len > data.length) break; - pos += len; + } catch (IOException e) { + return -1; } - if (pos > 0 - && pos + 4 <= data.length + int pos = data.length - bais.available(); + if (pos + 4 <= data.length && data[pos] == 'P' && data[pos + 1] == 'A' && data[pos + 2] == 'C' @@ -131,14 +135,6 @@ private static int findPackSignature(byte[] data) { return -1; } - private static int parsePacketLength(byte[] data, int pos) { - try { - return Integer.parseInt(new String(data, pos, 4, StandardCharsets.US_ASCII), 16); - } catch (NumberFormatException e) { - return -1; - } - } - private static PackEntry parseEntry(byte[] data, int pos) throws IOException { if (pos >= data.length) { return null; diff --git a/git-proxy-java-core/src/main/java/org/finos/gitproxy/servlet/filter/EnrichPushCommitsFilter.java b/git-proxy-java-core/src/main/java/org/finos/gitproxy/servlet/filter/EnrichPushCommitsFilter.java index b294f8cd..6a0f128b 100644 --- a/git-proxy-java-core/src/main/java/org/finos/gitproxy/servlet/filter/EnrichPushCommitsFilter.java +++ b/git-proxy-java-core/src/main/java/org/finos/gitproxy/servlet/filter/EnrichPushCommitsFilter.java @@ -6,7 +6,6 @@ import jakarta.servlet.http.HttpServletResponse; import java.io.ByteArrayInputStream; import java.io.IOException; -import java.nio.charset.StandardCharsets; import java.util.List; import java.util.Set; import lombok.extern.slf4j.Slf4j; @@ -16,6 +15,7 @@ import org.eclipse.jgit.transport.PackParser; import org.finos.gitproxy.git.Commit; import org.finos.gitproxy.git.CommitInspectionService; +import org.finos.gitproxy.git.GitReceivePackParser; import org.finos.gitproxy.git.GitRequestDetails; import org.finos.gitproxy.git.HttpOperation; import org.finos.gitproxy.git.LocalRepositoryCache; @@ -114,7 +114,7 @@ private void unpackPushData(HttpServletRequest request, Repository repository) t } // Walk past pkt-lines to find the PACK data boundary - int packOffset = findPackDataOffset(body); + int packOffset = GitReceivePackParser.findPackDataOffset(body); if (packOffset < 0) { log.debug("No PACK signature found in request body"); return; @@ -149,55 +149,6 @@ private byte[] getRequestBody(HttpServletRequest request) { return null; } - /** - * Find the start of PACK data by walking past the pkt-line section. The git receive-pack request body is: - * - *
- * pkt-line(s): 4-hex-digit length prefix + data (ref updates + capabilities) - * flush: 0000 - * PACK data: PACK + version + object count + objects... - *- * - *
CVE-2025-54584: replaces the former naive byte-scan for 'P','A','C','K' which could be spoofed by a crafted - * ref name (e.g. {@code refs/heads/PACK-evil}). - */ - private int findPackDataOffset(byte[] data) { - int pos = 0; - while (pos + 4 <= data.length) { - int len = parsePacketLength(data, pos); - if (len < 0) { - // Not a valid pkt-line hex prefix — assume we've reached pack data - break; - } - if (len == 0) { - // Flush packet (0000) — pack data starts immediately after - pos += 4; - break; - } - if (len < 4 || pos + len > data.length) { - break; - } - pos += len; - } - // Verify the PACK signature is at the expected position - if (pos + 4 <= data.length - && data[pos] == 'P' - && data[pos + 1] == 'A' - && data[pos + 2] == 'C' - && data[pos + 3] == 'K') { - return pos; - } - return -1; - } - - private static int parsePacketLength(byte[] data, int pos) { - try { - return Integer.parseInt(new String(data, pos, 4, StandardCharsets.US_ASCII), 16); - } catch (NumberFormatException e) { - return -1; - } - } - private void addFallbackCommit(GitRequestDetails requestDetails) { if (requestDetails.getCommit() != null) { requestDetails.getPushedCommits().add(requestDetails.getCommit());