From b19021ade3d0b71c89d35cb00eb9e589a121faa5 Mon Sep 17 00:00:00 2001 From: Helena Mariano <31138349+helenamariano@users.noreply.github.com> Date: Thu, 7 Jul 2022 14:16:47 +1000 Subject: [PATCH] Fix remote code execution vulnerability in the PDF OCR converter (#110) ENG-3252 --- pdf_ocr.go | 24 +++++++++++++++++++++++- pdf_ocr_test.go | 30 ++++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+), 1 deletion(-) create mode 100644 pdf_ocr_test.go diff --git a/pdf_ocr.go b/pdf_ocr.go index 4d9d53a..a05c943 100644 --- a/pdf_ocr.go +++ b/pdf_ocr.go @@ -1,3 +1,4 @@ +//go:build ocr // +build ocr package docconv @@ -10,6 +11,7 @@ import ( "os" "os/exec" "path/filepath" + "regexp" "strings" "sync" ) @@ -111,7 +113,8 @@ func ConvertPDFImages(path string) (BodyResult, error) { // PdfHasImage verify if `path` (PDF) has images func PDFHasImage(path string) bool { cmd := "pdffonts -l 5 %s | tail -n +3 | cut -d' ' -f1 | sort | uniq" - out, err := exec.Command("bash", "-c", fmt.Sprintf(cmd, path)).Output() + out, err := exec.Command("bash", "-c", fmt.Sprintf(cmd, shellEscape(path))).CombinedOutput() + if err != nil { log.Println(err) return false @@ -159,3 +162,22 @@ func ConvertPDF(r io.Reader) (string, map[string]string, error) { return fullBody, metaResult.meta, nil } + +var shellEscapePattern *regexp.Regexp + +func init() { + shellEscapePattern = regexp.MustCompile(`[^\w@%+=:,./-]`) +} + +// shellEscape returns a shell-escaped version of the string s. The returned value +// is a string that can safely be used as one token in a shell command line. +func shellEscape(s string) string { + if len(s) == 0 { + return "''" + } + if shellEscapePattern.MatchString(s) { + return "'" + strings.Replace(s, "'", "'\"'\"'", -1) + "'" + } + + return s +} diff --git a/pdf_ocr_test.go b/pdf_ocr_test.go new file mode 100644 index 0000000..73a0af5 --- /dev/null +++ b/pdf_ocr_test.go @@ -0,0 +1,30 @@ +//go:build ocr +// +build ocr + +package docconv + +import ( + "os" + "testing" +) + +func TestPDFHasImage_CannotExecuteCode(t *testing.T) { + // Try to inject code by passing a bad file path. + // If the code was successful it will create a file called foo in the working directory + badFilePath := "$(id >> foo).pdf" + if got, want := PDFHasImage(badFilePath), false; got != want { + t.Errorf("got %v, want %v", got, want) + } + + if got, want := fileExists("foo"), false; got != want { + t.Errorf("got bad file exists, want not file to exist") + } +} + +func fileExists(filename string) bool { + info, err := os.Stat(filename) + if os.IsNotExist(err) { + return false + } + return !info.IsDir() +}