Skip to content

Commit

Permalink
Make semgrep happy
Browse files Browse the repository at this point in the history
  • Loading branch information
tgulacsi committed Aug 28, 2022
1 parent ee44a92 commit ff82200
Show file tree
Hide file tree
Showing 15 changed files with 99 additions and 55 deletions.
6 changes: 3 additions & 3 deletions centos/install.sh
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ set -e
sudo useradd -l agostle
yum install libreoffice GraphicsMagick ghostscript
if ! yum install pdftk-1.44-2.el6.rf.x86_64.rpm runit-2.1.1-6.el6.x86_64.rpm; then
. $(dirname $0)/install_pdftk.sh
. $(dirname $0)/install_runit.sh
. "$(dirname "$0")/install_pdftk.sh"
. "$(dirname "$0")/install_runit.sh"
fi
cp -pr $(dirname $0)/etc/service/agostle /etc/service/
cp -pr "$(dirname "$0")/etc/service/agostle" /etc/service/
7 changes: 5 additions & 2 deletions converter/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ func LoadConfig(ctx context.Context, fn string) error {
}
var err error
cd := filepath.Join(Workdir, "agostle-filecache")
// nosemgrep: go.lang.correctness.permissions.file_permission.incorrect-default-permission
_ = os.MkdirAll(cd, 0700)
if Cache, err = filecache.Open(cd); err != nil {
var tErr error
Expand All @@ -122,6 +123,7 @@ func LoadConfig(ctx context.Context, fn string) error {
bn := filepath.Base(*ConfPdfseparate)
prefix := (*ConfPdfseparate)[:len(*ConfPdfseparate)-len(bn)]
for k := range popplerOk {
// nosemgrep: go.lang.security.audit.dangerous-exec-command.dangerous-exec-command
if err := exec.CommandContext(ctx, prefix+k, "-h").Run(); err == nil {
popplerOk[k] = prefix + k
}
Expand All @@ -144,10 +146,10 @@ var Cache *filecache.Cache
// LeaveTempFiles should be true only for debugging purposes (leaves temp files)
var LeaveTempFiles = false

type ctxKey string
type ctxKeyWorkDir struct{}

func prepareContext(ctx context.Context, subdir string) (context.Context, string) {
const wdKey = ctxKey("workdir")
var wdKey ctxKeyWorkDir
odir, _ := ctx.Value(wdKey).(string)
if odir != "" {
if subdir != "" {
Expand All @@ -162,6 +164,7 @@ func prepareContext(ctx context.Context, subdir string) (context.Context, string
}
ndir, ok := ctx.Value(wdKey).(string)
if ok && odir != ndir {
// nosemgrep: go.lang.correctness.permissions.file_permission.incorrect-default-permission
if err := os.MkdirAll(ndir, 0750); err != nil {
panic("cannot create workdir " + ndir + ": " + err.Error())
}
Expand Down
2 changes: 2 additions & 0 deletions converter/converter.go
Original file line number Diff line number Diff line change
Expand Up @@ -400,6 +400,7 @@ func lofficeConvert(ctx context.Context, outDir, inpfn string) error {
defer lofficePortLock.Unlock()
}
subCtx, subCancel := context.WithTimeout(ctx, *ConfLofficeTimeout)
// nosemgrep: go.lang.security.audit.dangerous-exec-command.dangerous-exec-command
cmd := exec.CommandContext(subCtx, *ConfLoffice, args...)
cmd.Dir = filepath.Dir(inpfn)
cmd.Stderr = os.Stderr
Expand Down Expand Up @@ -468,6 +469,7 @@ func wkhtmltopdf(ctx context.Context, outfn, inpfn string) error {
"--user-style-sheet", ussFn,
outfn}
var buf bytes.Buffer
// nosemgrep: go.lang.security.audit.dangerous-exec-command.dangerous-exec-command
cmd := exec.CommandContext(ctx, *ConfWkhtmltopdf, args...)
cmd.Dir = filepath.Dir(inpfn)
cmd.Stderr = &buf
Expand Down
11 changes: 6 additions & 5 deletions converter/fileutils.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ package converter
import (
"archive/zip"
"bytes"
"crypto/sha1"
"crypto/sha512"
"errors"
"fmt"
"hash"
Expand Down Expand Up @@ -159,8 +159,7 @@ func (a ArchFileItem) ArchiveName() string {
return a.Filename
}
if a.File != nil {
fi, err := a.File.Stat()
if err != nil {
if fi, err := a.File.Stat(); err == nil {
return fi.Name()
}
}
Expand Down Expand Up @@ -256,14 +255,16 @@ func zipFiles(dest io.Writer, skipOnError, unsafeArchFn bool, files <-chan ArchF
continue
}
openedHere = true
if item.File, err = os.Open(item.Filename); err != nil {
f, err := os.Open(item.Filename)
if err != nil {
err = fmt.Errorf("zip cannot open %q: %w", item.Filename, err)
if !skipOnError {
return err
}
appendErr(err)
continue
}
item.File = f
}
if fi, err = item.File.Stat(); err != nil {
if openedHere {
Expand Down Expand Up @@ -376,7 +377,7 @@ func fileContentHash(fn string) (hash.Hash, error) {
if err != nil {
return nil, err
}
hsh := sha1.New()
hsh := sha512.New384()
_, err = io.Copy(hsh, f)
f.Close()
return hsh, err
Expand Down
24 changes: 15 additions & 9 deletions converter/filters.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ package converter
import (
"archive/zip"
"bytes"
"crypto/sha1"
"crypto/sha256"
"encoding/base64"
"errors"
"fmt"
Expand Down Expand Up @@ -348,11 +348,11 @@ func SlurpMail(ctx context.Context, partch chan<- i18nmail.MailPart, errch chan<
var head [4096]byte

logger.Info("SlurpMail", "ct", contentType)
mp := i18nmail.MailPart{ContentType: contentType}
var err error
if mp.Body, err = i18nmail.MakeSectionReader(body, bodyThreshold); err != nil {
sr, err := i18nmail.MakeSectionReader(body, bodyThreshold)
if err != nil {
return
}
mp := i18nmail.MailPart{Body: sr, ContentType: contentType}
b := make([]byte, 2048)
n, _ := mp.Body.ReadAt(b, 0)
b = b[:n]
Expand Down Expand Up @@ -416,7 +416,7 @@ func SlurpMail(ctx context.Context, partch chan<- i18nmail.MailPart, errch chan<
//close(errch)
}

const ctxSeen = ctxKey("seen")
type ctxKeySeen struct{}

// SetupFilters applies filters on parts received on inch, and returns them on outch
func SetupFilters(
Expand All @@ -428,7 +428,7 @@ func SetupFilters(
if len(Filters) == 0 {
return inch
}
ctx = context.WithValue(ctx, ctxSeen, make(map[string]int, 32))
ctx = context.WithValue(ctx, ctxKeySeen{}, make(map[string]int, 32))

in := inch
var out chan i18nmail.MailPart
Expand All @@ -447,7 +447,7 @@ const maxErrLen = 1 << 20
// all mail part goes through all filter in Filters, in reverse order (last first)
func MailToPdfFiles(ctx context.Context, r io.Reader, contentType string) (files []ArchFileItem, err error) {
logger := getLogger(ctx)
hsh := sha1.New()
hsh := sha256.New()
sr, e := iohlp.MakeSectionReader(io.TeeReader(r, hsh), 1<<20)
logger.Info("MailToPdfFiles", "input", sr.Size(), "error", e)
if e != nil {
Expand Down Expand Up @@ -613,6 +613,7 @@ func MailToTree(ctx context.Context, outdir string, r io.Reader) error {
strings.Replace(mp.ContentType, "/", "--", -1), xfn)
}

// nosemgrep: go.lang.correctness.permissions.file_permission.incorrect-default-permission
if err = os.MkdirAll(outdir, 0750); err != nil {
return fmt.Errorf("MailToTree(%q): %w", outdir, err)
}
Expand All @@ -639,6 +640,7 @@ func MailToTree(ctx context.Context, outdir string, r io.Reader) error {
upr = append(upr, up[i])
}
dn = filepath.Join(upr...)
// nosemgrep: go.lang.correctness.permissions.file_permission.incorrect-default-permission
_ = os.MkdirAll(dn, 0750)
break
}
Expand Down Expand Up @@ -706,6 +708,7 @@ func ExtractingFilter(ctx context.Context,
}()

allIn := make(chan i18nmail.MailPart, 1024)
// nosemgrep: trailofbits.go.waitgroup-add-called-inside-goroutine.waitgroup-add-called-inside-goroutine
var wg sync.WaitGroup // for waiting all input to finish
go func() {
for part := range inch {
Expand Down Expand Up @@ -733,9 +736,12 @@ func ExtractingFilter(ctx context.Context,
}
child := part.Spawn()
child.ContentType = messageRFC822
if child.Body, err = i18nmail.MakeSectionReader(r, bodyThreshold); err != nil {
body, bodyErr := i18nmail.MakeSectionReader(r, bodyThreshold)
if err != nil {
err = bodyErr
goto Error
}
child.Body = body
fn := headerGetFileName(part.Header)
if fn == "" {
fn = ".eml"
Expand Down Expand Up @@ -842,7 +848,7 @@ func DupFilter(ctx context.Context,
defer func() {
close(outch)
}()
seen := ctx.Value(ctxSeen).(map[string]int)
seen := ctx.Value(ctxKeySeen{}).(map[string]int)
if seen == nil {
seen = make(map[string]int, 32)
}
Expand Down
8 changes: 6 additions & 2 deletions converter/filters_html.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,8 @@ func HTMLPartFilter(ctx context.Context,
alter = ""
converter = GetConverter(textHtml, nil)
aConverter Converter
tbd = make(map[string]struct{}, 4)
// nosemgrep
tbd = make(map[string]struct{}, 4)
)
if !LeaveTempFiles {
defer func() {
Expand Down Expand Up @@ -76,11 +77,12 @@ func HTMLPartFilter(ctx context.Context,
err = converter(ctx, destfn, fh, textHtml)
fh.Close()
if err == nil {
return destfn, err
return destfn, nil
}
err = fmt.Errorf("converting %s to %s: %w", fn, destfn, err)
}
logger.Info("html2pdf", "error", err)
// nosemgrep: go.lang.correctness.useless-eqeq.eqeq-is-bad
if alter != "" && aConverter != nil {
logger.Info("html2pdf using alternative content " + alter)
if fh, err = os.Open(alter); err != nil {
Expand Down Expand Up @@ -140,6 +142,7 @@ func HTMLPartFilter(ctx context.Context,
} else {
dn = wd
}
// nosemgrep: go.lang.correctness.permissions.file_permission.incorrect-default-permission
_ = os.Mkdir(dn, 0755) //ignore errors
fn = ""
if parent != nil {
Expand Down Expand Up @@ -213,6 +216,7 @@ func HTMLPartFilter(ctx context.Context,
}
fn = filepath.Join(filepath.Dir(cg.htmlFn), fn)

// nosemgrep: go.lang.correctness.permissions.file_permission.incorrect-default-permission
_ = os.Mkdir(filepath.Dir(fn), 0755) // ignore error
logger.Info("save", "file", fn, "cid", cid, "htmlFn", cg.htmlFn)
_, _ = part.Body.Seek(0, 0)
Expand Down
1 change: 1 addition & 0 deletions converter/image.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ func command(ctx context.Context, prg string, args ...string) *exec.Cmd {
if prg == "" {
prg, args = args[0], args[1:]
}
// nosemgrep: go.lang.security.audit.dangerous-exec-command.dangerous-exec-command
return exec.CommandContext(ctx, prg, args...)
}

Expand Down
19 changes: 14 additions & 5 deletions converter/pdf.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ package converter

import (
"bytes"
"crypto/sha1"
"crypto/sha256"
"encoding/base64"
"encoding/gob"
"errors"
Expand Down Expand Up @@ -64,9 +64,11 @@ func pdfPageNum(ctx context.Context, srcfn string) (numberofpages int, encrypted
pdfinfo := false
var cmd *exec.Cmd
if popplerOk["pdfinfo"] != "" {
// nosemgrep: go.lang.security.audit.dangerous-exec-command.dangerous-exec-command
cmd = exec.CommandContext(ctx, popplerOk["pdfinfo"], srcfn)
pdfinfo = true
} else {
// nosemgrep: go.lang.security.audit.dangerous-exec-command.dangerous-exec-command
cmd = exec.CommandContext(ctx, *ConfPdftk, srcfn, "dump_data_utf8")
}
out, e := cmd.CombinedOutput()
Expand Down Expand Up @@ -220,7 +222,7 @@ func PdfSplit(ctx context.Context, srcfn string, pages []uint16) (filenames []st
}
}
//logger.Info("", "prefix", prefix, "fn", fn, "i", i)
n, iErr := strconv.Atoi(fn[len(prefix) : len(prefix)+i])
n, iErr := strconv.ParseUint(fn[len(prefix):len(prefix)+i], 10, 16)
if iErr != nil {
err = fmt.Errorf("%q: %w", fn, iErr)
return
Expand Down Expand Up @@ -322,6 +324,7 @@ func pdfMerge(ctx context.Context, destfn string, filenames ...string) error {
if pdfunite != "" {
args := append(append(make([]string, 0, len(filenames)+1), filenames...),
destfn)
// nosemgrep: go.lang.security.audit.dangerous-exec-command.dangerous-exec-command
cmd := exec.CommandContext(ctx, pdfunite, args...)
cmd.Stdout = io.MultiWriter(&buf, os.Stdout)
cmd.Stderr = io.MultiWriter(&buf, os.Stderr)
Expand All @@ -334,6 +337,7 @@ func pdfMerge(ctx context.Context, destfn string, filenames ...string) error {
}
args := append(append(make([]string, 0, len(filenames)+3), filenames...),
"cat", "output", destfn)
// nosemgrep: go.lang.security.audit.dangerous-exec-command.dangerous-exec-command
cmd := exec.CommandContext(ctx, *ConfPdftk, args...)
cmd.Stdout = io.MultiWriter(&buf, os.Stdout)
cmd.Stderr = io.MultiWriter(&buf, os.Stderr)
Expand All @@ -356,7 +360,7 @@ func getHash(fn string) string {
logger.Info("WARN getHash open", "fn", fn, "error", err)
return ""
}
hsh := sha1.New()
hsh := sha256.New()
_, err = io.Copy(hsh, fh)
_ = fh.Close()
if err != nil {
Expand All @@ -366,11 +370,12 @@ func getHash(fn string) string {
}

func isAlreadyCleaned(fn string) bool {
var err error
if !filepath.IsAbs(fn) {
if fn, err = filepath.Abs(fn); err != nil {
afn, err := filepath.Abs(fn)
if err != nil {
logger.Info("WARN cannot absolutize filename", "fn", fn, "error", err)
}
fn = afn
}
cleanMtx.Lock()
defer cleanMtx.Unlock()
Expand Down Expand Up @@ -466,6 +471,7 @@ func PdfClean(ctx context.Context, fn string) (err error) {
}

func call(ctx context.Context, what string, args ...string) error {
// nosemgrep: go.lang.security.audit.dangerous-exec-command.dangerous-exec-command
cmd := exec.CommandContext(ctx, what, args...)
if cmd.Stderr == nil {
cmd.Stderr = os.Stderr
Expand All @@ -477,6 +483,7 @@ func call(ctx context.Context, what string, args ...string) error {
}

func callAt(ctx context.Context, what, where string, args ...string) error {
// nosemgrep: go.lang.security.audit.dangerous-exec-command.dangerous-exec-command
cmd := exec.CommandContext(ctx, what, args...)
cmd.Stderr = os.Stderr
cmd.Dir = where
Expand Down Expand Up @@ -558,6 +565,7 @@ func PdfRewrite(ctx context.Context, destfn, srcfn string) error {
// PdfDumpFields dumps the field names from the given PDF.
func PdfDumpFields(ctx context.Context, inpfn string) ([]string, error) {
var buf bytes.Buffer
// nosemgrep: go.lang.security.audit.dangerous-exec-command.dangerous-exec-command
cmd := exec.CommandContext(ctx, *ConfPdftk, inpfn, "dump_data_fields_utf8", "output", "-")
cmd.Stdout = &buf
if err := cmd.Run(); err != nil {
Expand Down Expand Up @@ -601,6 +609,7 @@ func PdfFillFdf(ctx context.Context, destfn, inpfn string, values map[string]str
return err
}

// nosemgrep: go.lang.security.audit.dangerous-exec-command.dangerous-exec-command
cmd := exec.CommandContext(ctx, *ConfPdftk, inpfn, "fill_form", "-", "output", destfn)
cmd.Stdin = bytes.NewReader(buf.Bytes())
return execute(cmd)
Expand Down
5 changes: 3 additions & 2 deletions converter/portlock.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,11 @@ func NewPortLock(port int) *PortLock {

// Lock locks on port
func (p *PortLock) Lock() {
var err error
t := 1 * time.Second
for {
if p.ln, err = net.Listen("tcp", p.hostport); err == nil {
ln, err := net.Listen("tcp", p.hostport)
if err == nil {
p.ln = ln
return
}
logger.Info("spinning lock hostport", "hostport", p.hostport, "error", err)
Expand Down
4 changes: 2 additions & 2 deletions email_convert.go
Original file line number Diff line number Diff line change
Expand Up @@ -129,11 +129,11 @@ func emailConvertDecode(ctx context.Context, r *http.Request) (interface{}, erro
break
}
}
var err error
req.Input, err = getOneRequestFile(ctx, r)
inp, err := getOneRequestFile(ctx, r)
if err != nil {
return nil, err
}
req.Input = inp
//getLogger(ctx).Log("input", req.Input)
contentType := req.Input.Header.Get("Content-Type")
if contentType == "" || contentType == "application/octet-stream" {
Expand Down
Loading

0 comments on commit ff82200

Please sign in to comment.