From d7f304045af2f602c5b0c1a26d2c7f643753cf8f Mon Sep 17 00:00:00 2001 From: Michael Dardis Date: Wed, 4 Mar 2020 15:25:09 +1100 Subject: [PATCH] Add validation to hex decoder Does not fix any issue but prevents user from shooting themselves in the foot with improper configuration. Suggested by Demetr Starshov at https://git.eclipse.org/r/#/c/157681/ Change-Id: I006d65022f0a7d4066970825d00080c59404fdc3 Signed-off-by: Michael Dardis Signed-off-by: Matthias Sohn --- .../tst/org/eclipse/jgit/util/HexTest.java | 25 +++++++++++++++++++ .../eclipse/jgit/internal/JGitText.properties | 1 + .../org/eclipse/jgit/internal/JGitText.java | 1 + .../src/org/eclipse/jgit/util/Hex.java | 21 +++++++++++++--- 4 files changed, 45 insertions(+), 3 deletions(-) diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/util/HexTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/util/HexTest.java index 32af07f25..5f3ab0b77 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/util/HexTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/util/HexTest.java @@ -42,6 +42,31 @@ public void testEncodeMatchesDecode() { } } + @Test(expected = IllegalArgumentException.class) + public void testIllegal() { + decode("0011test00"); + } + + @Test(expected = IllegalArgumentException.class) + public void testIllegal2() { + decode("0123456789abcdefgh"); + } + + @Test(expected = IllegalArgumentException.class) + public void testIllegal3() { + decode("0123456789abcdef-_+*"); + } + + @Test + public void testLegal() { + decode("0123456789abcdef"); + } + + @Test + public void testLegal2() { + decode("deadbeef"); + } + private static byte[] b(String str) { return Constants.encode(str); } 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 153399ca3..1218ee612 100644 --- a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties +++ b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties @@ -354,6 +354,7 @@ invalidFilter=Invalid filter: {0} invalidGitdirRef = Invalid .git reference in file ''{0}'' invalidGitModules=Invalid .gitmodules file invalidGitType=invalid git type: {0} +invalidHexString=Invalid hex string: {0} invalidHooksPath=Invalid git config core.hooksPath = {0} invalidId=Invalid id: {0} invalidId0=Invalid id 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 ef4c85594..6235dd83d 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java @@ -382,6 +382,7 @@ public static JGitText get() { /***/ public String invalidGitdirRef; /***/ public String invalidGitModules; /***/ public String invalidGitType; + /***/ public String invalidHexString; /***/ public String invalidHooksPath; /***/ public String invalidId; /***/ public String invalidId0; diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/util/Hex.java b/org.eclipse.jgit/src/org/eclipse/jgit/util/Hex.java index 935903652..925159830 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/util/Hex.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/util/Hex.java @@ -10,6 +10,10 @@ package org.eclipse.jgit.util; +import java.text.MessageFormat; + +import org.eclipse.jgit.internal.JGitText; + /** * Encodes and decodes to and from hexadecimal notation. * @@ -27,9 +31,11 @@ private Hex() { /** * Decode a hexadecimal string to a byte array. * - * Note this method performs no validation on input content. + * Note this method validates that characters in the given string are valid + * as digits in a hex string. * - * @param s hexadecimal string + * @param s + * hexadecimal string * @return decoded array */ public static byte[] decode(String s) { @@ -37,7 +43,16 @@ public static byte[] decode(String s) { byte[] b = new byte[len / 2]; for (int i = 0; i < len; i += 2) { - b[i / 2] = (byte) ((Character.digit(s.charAt(i), 16) << 4) | Character.digit(s.charAt(i + 1), 16)); + int left = Character.digit(s.charAt(i), 16); + int right = Character.digit(s.charAt(i + 1), 16); + + if (left == -1 || right == -1) { + throw new IllegalArgumentException(MessageFormat.format( + JGitText.get().invalidHexString, + s)); + } + + b[i / 2] = (byte) (left << 4 | right); } return b; }