Make ParsePatch more robust (#17573)

diff --git a/services/gitdiff/gitdiff.go b/services/gitdiff/gitdiff.go
index f843bc4dcf9dc45ecdcdb3f95be5a3cac15e9f9e..614f8104ecaf2d9c2ac5b14fa482e7b5bd0d391c 100644 (file)
--- a/services/gitdiff/gitdiff.go
+++ b/services/gitdiff/gitdiff.go
@@ -839,7 +839,12 @@ parsingLoop:
                        case strings.HasPrefix(line, "--- "):
                                // Handle ambiguous filenames
                                if curFile.IsAmbiguous {
-                                       if len(line) > 6 && line[4] == 'a' {
+                                       // The shortest string that can end up here is:
+                                       // "--- a\t\n" without the qoutes.
+                                       // This line has a len() of 7 but doesn't contain a oldName.
+                                       // So the amount that the line need is at least 8 or more.
+                                       // The code will otherwise panic for a out-of-bounds.
+                                       if len(line) > 7 && line[4] == 'a' {
                                                curFile.OldName = line[6 : len(line)-1]
                                                if line[len(line)-2] == '\t' {
                                                        curFile.OldName = curFile.OldName[:len(curFile.OldName)-1]
@@ -1194,6 +1199,11 @@ func readFileName(rd *strings.Reader) (string, bool) {
        _ = rd.UnreadByte()
        if char == '"' {
                fmt.Fscanf(rd, "%q ", &name)
+               if len(name) == 0 {
+                       log.Error("Reader has no file name: %v", rd)
+                       return "", true
+               }
+
                if name[0] == '\\' {
                        name = name[1:]
                }

diff --git a/services/gitdiff/gitdiff_test.go b/services/gitdiff/gitdiff_test.go
index d69d0c01d8df8827928d5f00e2ded7315da81313..6decb59b64b5a91864bad93f4430e0a3b2e99784 100644 (file)
--- a/services/gitdiff/gitdiff_test.go
+++ b/services/gitdiff/gitdiff_test.go
@@ -541,3 +541,22 @@ func TestDiffToHTML_14231(t *testing.T) {
 
        assertEqual(t, expected, output)
 }
+
+func TestNoCrashes(t *testing.T) {
+       type testcase struct {
+               gitdiff string
+       }
+
+       tests := []testcase{
+               {
+                       gitdiff: "diff --git \n--- a\t\n",
+               },
+               {
+                       gitdiff: "diff --git \"0\n",
+               },
+       }
+       for _, testcase := range tests {
+               // It shouldn't crash, so don't care about the output.
+               ParsePatch(setting.Git.MaxGitDiffLines, setting.Git.MaxGitDiffLineCharacters, setting.Git.MaxGitDiffFiles, strings.NewReader(testcase.gitdiff))
+       }
+}