From 0d5d18e6d6d87c423b92066a299989f06e0103ef Mon Sep 17 00:00:00 2001 From: Raja Boujbel Date: Fri, 24 Mar 2023 12:56:32 +0100 Subject: [PATCH] lint: add E73 for relative path on extra-files --- master_changes.md | 2 ++ src/state/opamFileTools.ml | 16 +++++++++++++++- tests/reftests/extrafile.test | 2 ++ tests/reftests/lint.test | 28 +++++++++++++++++++++++++++- 4 files changed, 46 insertions(+), 2 deletions(-) diff --git a/master_changes.md b/master_changes.md index c566d454d37..5586e2526b0 100644 --- a/master_changes.md +++ b/master_changes.md @@ -72,6 +72,7 @@ users) * Add E70 to check `extra-files:` duplicated fields [#5561 @rjbou] * Add E71 to check if the same checksum algorithm is used several times for a given url in `url` section [#5561 @rjbou] * Add E72 to check if the same checksum algorithm is used several times for a given url in `extra-sources` section [#5561 @rjbou] + * Add E73 to check that paths in `extra-files:` are not escapable [#5561 @rjbou] ## Repository * Mitigate curl/curl#13845 by falling back from --write-out to --fail if exit code 43 is returned by curl [#6168 @dra27 - fix #6120] @@ -185,6 +186,7 @@ users) * lint: add E70 test [#5561 @rjbou] * lint: add E71 test [#5561 @rjbou] * lint: add E72 test [#5561 @rjbou] + * lint: add E73 test [#5561 @rjbou] ### Engine * Add a test filtering mechanism [#6105 @Keryan-dev] diff --git a/src/state/opamFileTools.ml b/src/state/opamFileTools.ml index a860e147952..ced443f74a8 100644 --- a/src/state/opamFileTools.ml +++ b/src/state/opamFileTools.ml @@ -930,7 +930,7 @@ let t_lint ?check_extra_files ?(check_upstream=false) ?(all=false) t = ("file" | "path" | "local" | "rsync") -> true | _, _ -> false) && (Filename.is_relative u.path - || OpamStd.String.contains ~sub:".." u.path)) + || OpamFilename.might_escape ~sep:`Unix u.path)) (all_urls t) in cond 65 `Error @@ -1078,6 +1078,20 @@ let t_lint ?check_extra_files ?(check_upstream=false) ?(all=false) t = cond 72 `Error "Field 'extra-sources' contains duplicated checksums" ?detail has_double); + (let relative = + match t.extra_files with + | None -> [] + | Some extra_files -> + List.filter_map (fun (base, _) -> + let path = OpamFilename.Base.to_string base in + if OpamFilename.might_escape ~sep:`Unix path then + Some path else None) + extra_files + in + cond 73 `Error + "Field 'extra-files' contains path with '..'" + ~detail:relative + (relative <> [])); ] in format_errors @ diff --git a/tests/reftests/extrafile.test b/tests/reftests/extrafile.test index 8ab8e312d66..cff98501671 100644 --- a/tests/reftests/extrafile.test +++ b/tests/reftests/extrafile.test @@ -528,10 +528,12 @@ Clearing cache of downloaded files ### opam lint --package escape-good-md5 /escape-good-md5.1: Errors. error 53: Mismatching 'extra-files:' field: "../../../no-checksum/no-checksum.1/files/p.patch" + error 73: Field 'extra-files' contains path with '..': "../../../no-checksum/no-checksum.1/files/p.patch" # Return code 1 # ### opam lint --package escape-good-md5 --check-upstream /escape-good-md5.1: Errors. error 53: Mismatching 'extra-files:' field: "../../../no-checksum/no-checksum.1/files/p.patch" + error 73: Field 'extra-files' contains path with '..': "../../../no-checksum/no-checksum.1/files/p.patch" # Return code 1 # ### # ERROR it copies it to build dir ^ relative path -> escape!!!! ### # currently writing in /tmp as a common it copies in diff --git a/tests/reftests/lint.test b/tests/reftests/lint.test index 54fa33e1851..c8ac4ac4991 100644 --- a/tests/reftests/lint.test +++ b/tests/reftests/lint.test @@ -952,7 +952,7 @@ pin-depends: [ ] ### opam lint ./lint.opam ${BASEDIR}/lint.opam: Errors. - error 65: URLs must be absolute: "./my/url/path", "/wrong/../mirror", "./../to@li.nt", "/my/./../extrasource/path", "/my/extrasource..path.patch", "/my/../pinned/package" + error 65: URLs must be absolute: "./my/url/path", "/wrong/../mirror", "./../to@li.nt", "/my/./../extrasource/path", "/my/../pinned/package" # Return code 1 # ### : W66: String that can't be resolved to bool in filtered package formula ### @@ -1157,3 +1157,29 @@ extra-source "double-file3" { ${BASEDIR}/lint.opam: Errors. error 72: Field 'extra-sources' contains duplicated checksums: "double-file have md5: 2 occurences and sha256: 3 occurences", "double-file2 have md5: 2 occurences" # Return code 1 # +### : E73: Field 'extra-files' contains path with '..' +### +opam-version: "2.0" +synopsis: "A word" +description: "Two words." +authors: "the testing team" +homepage: "egapemoh" +maintainer: "maint@tain.er" +license: "ISC" +dev-repo: "hg+https://to@li.nt" +bug-reports: "https://nobug" +extra-source "double-file" { + src:"https://git.url/repo.tgz" + checksum:"md5=00000000000000000000000000000000" +} +extra-files: [ + [ "./relative/path" "md5=00000000000000000000000000000000"] + [ "/absolute/../relative/path" "md5=00000000000000000000000000000000"] + [ "/absolute/path" "md5=00000000000000000000000000000000"] + [ "extra-files..patch.patch" "md5=00000000000000000000000000000000"] +] +### opam lint ./lint.opam +${BASEDIR}/lint.opam: Errors. + error 53: Mismatching 'extra-files:' field: "./relative/path", "/absolute/../relative/path", "/absolute/path", "extra-files..patch.patch" + error 73: Field 'extra-files' contains path with '..': "/absolute/../relative/path" +# Return code 1 #