sdformat
sdformat copied to clipboard
sdf::findFile does not sanitize input filename, when given a path instead of a file name.
Environment
- OS Version: Ubuntu 20.04
- Source build, branch
sdf11
Description
- Expected behavior:
When calling sdf::findFile
with test_model/model.sdf
as the input _filename
, instead of a base file name (e.g. model.sdf
), it does not throw any exceptions and returns the full path /path/to/test_model/model.sdf
.
- Actual behavior:
In Windows machines however, we will produce a path like so C:/somewhere\\path\\to\\models\\test_model/model.sdf
, due to the lack of path sanitization.
Output
This was how it looked like during the Windows CI build for the lines https://github.com/osrf/sdformat/blob/sdf11/test/sdf/includes.sdf#L17-L20,
17: [ RUN ] ElementTracing.includes
17: C:\Jenkins\workspace\sdformat-ci-pr_any-windows7-amd64\ws\sdformat\test\integration\element_tracing.cc(283): error: Expected equality of these values:
17: modelFilePath
17: Which is: "C:/Jenkins/workspace/sdformat-ci-pr_any-windows7-amd64/ws/sdformat\\test\\integration\\model\\test_model\\model.sdf"
17: overrideModelWithFileElem->FilePath()
17: Which is: "C:/Jenkins/workspace/sdformat-ci-pr_any-windows7-amd64/ws/sdformat\\test\\integration\\model\\test_model/model.sdf"
17: [ FAILED ] ElementTracing.includes (201 ms)
I thought the issue was actually in sdf::findFile
when it's used to find the included file: https://github.com/osrf/sdformat/blob/e5ca000c1934b59302362ef85016ae20af501ed0/src/parser.cc#L1103-L1106
In your example, doesn't sdf::Element::FilePath()
return C:/somewhere\\path\\to\\models\\test_model/model.sdf
?
Yup, it did. Sorry I got confused myself for a moment. I just did some basic tests, and it is as you mentioned, it was sdf::findFile
in SDFImpl.hh
, https://github.com/osrf/sdformat/blob/sdf11/include/sdf/SDFImpl.hh#L82-L99 that does not sanitize the path.
I will make the changes to the issue accordingly, sorry for the confusion.