[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Bug-wget] [PATCH 11/25] New: Metalink/XML and Metalink/HTTP file na
From: |
Matthew White |
Subject: |
Re: [Bug-wget] [PATCH 11/25] New: Metalink/XML and Metalink/HTTP file naming safety rules |
Date: |
Tue, 13 Sep 2016 05:30:11 +0200 |
On Sun, 11 Sep 2016 22:48:55 +0200
Giuseppe Scrivano <address@hidden> wrote:
> Matthew White <address@hidden> writes:
>
> > [Coverity Scan is ok, make syntax-check is ok, make check-valgrind is ok,
> > contrib/check-hard is ok]
> >
> > This introduces new rules/tests about Metalink/XML and Metalink/HTTP.
> >
> > The safety mechanism introduced provides secure and predictable file names.
> > This is convenient to prevent the overwriting of system/critical files and
> > to prevent to write files into unexpected/protected locations.
> >
> > The option --trust-server-names may be used to trust metalink:file names
> > when downloading files.
> >
> > Verbatim from doc/metalink-standard.txt:
> > ----------------------------------------
> > The final name of downloaded files is computed starting from a trusted
> > name, which is then combined with the "Directory Options". The result
> > is verified and eventually made safer following security rules. If the
> > final name isn't found safe enough, then the file isn't downloaded.
> >
> > Depending on the options used, a suffix could be appended to the final
> > name to not overwrite existing files.
> > ----------------------------------------
> >
> > Regards,
> > Matthew
> >
> > --
> > Matthew White <address@hidden>
> >
> > From 7fef56c4f815359047347eee4356b9f8971f1df5 Mon Sep 17 00:00:00 2001
> > From: Matthew White <address@hidden>
> > Date: Sun, 21 Aug 2016 18:45:09 +0200
> > Subject: [PATCH 11/25] New: Metalink/XML and Metalink/HTTP file naming
> > safety
> > rules
> >
> > * src/metalink.h: Add declaration of function append_suffix_number()
> > * src/metalink.c: Add function append_suffix_number() append number to
> > string
> > * src/metalink.c (retrieve_from_metalink): Safer Metalink/XML and
> > Metalink/HTTP download naming system, opt.trustservernames based
> > * doc/metalink-standard.txt: Update doc. Explain new Metalink/XML and
> > Metalin/HTTP download naming system and --trust-server-names role
> > * testenv/Makefile.am: Add new files
> > * testenv/Test-metalink-xml.py: Update test. Metalink/XML naming tests
> > * testenv/Test-metalink-xml-trust.py: New file. Metalink/XML naming
> > tests with --trust-server-names
> > * testenv/Test-metalink-xml-abspath.py: Update test. Metalink/XML
> > absolute path tests
> > * testenv/Test-metalink-xml-abspath-trust.py: New file. Metalink/XML
> > absolute path tests with --trust-server-names
> > * testenv/Test-metalink-xml-relpath.py: Update test. Metalink/XML
> > relative path tests
> > * testenv/Test-metalink-xml-relpath-trust.py: New file. Metalink/XML
> > relative path tests with --trust-server-names
> > * testenv/Test-metalink-xml-homepath.py: Update test. Metalink/XML
> > home path and ~ (tilde) tests
> > * testenv/Test-metalink-xml-homepath-trust.py: New file. Metalink/XML
> > home path and ~ (tilde) tests with --trust-server-names
> > * testenv/Test-metalink-xml-prefix.py: New file. Metalink/XML naming
> > tests with --directory-prefix
> > * testenv/Test-metalink-xml-prefix-trust.py: New file. Metalink/XML
> > naming tests with --directory-prefix and --trust-server-names
> > * testenv/Test-metalink-xml-absprefix.py: New file. Metalink/XML
> > absolute --directory-prefix tests
> > * testenv/Test-metalink-xml-absprefix-trust.py: New file. Metalink/XML
> > absolute --directory-prefix tests with --trust-server-names
> > * testenv/Test-metalink-xml-relprefix.py: New file. Metalink/XML
> > relative --directory-prefix tests
> > * testenv/Test-metalink-xml-relprefix-trust.py: New file. Metalink/XML
> > relative --directory-prefix tests with --trust-server-names
> > * testenv/Test-metalink-xml-homeprefix.py: New file. Metalink/XML home
> > --directory-prefix tests
> > * testenv/Test-metalink-xml-homeprefix-trust.py: New file. Metalink/XML
> > home --directory-prefix tests with --trust-server-names
> >
> > The option --trust-server-names allows to use the file names parsed
> > from a Metalink/XML file. Without --trust-server-names, the safety
> > mechanism provides secure and predictable file names.
> > ---
> > doc/metalink-standard.txt | 59 ++++++--
> > src/metalink.c | 107 +++++++++++---
> > src/metalink.h | 1 +
> > testenv/Makefile.am | 14 +-
> > testenv/Test-metalink-xml-abspath-trust.py | 130 +++++++++++++++++
> > testenv/Test-metalink-xml-abspath.py | 61 ++++++--
> > testenv/Test-metalink-xml-absprefix-trust.py | 194
> > +++++++++++++++++++++++++
> > testenv/Test-metalink-xml-absprefix.py | 194
> > +++++++++++++++++++++++++
> > testenv/Test-metalink-xml-homepath-trust.py | 195
> > +++++++++++++++++++++++++
> > testenv/Test-metalink-xml-homepath.py | 126 ++++++++++++++--
> > testenv/Test-metalink-xml-homeprefix-trust.py | 194
> > +++++++++++++++++++++++++
> > testenv/Test-metalink-xml-homeprefix.py | 194
> > +++++++++++++++++++++++++
> > testenv/Test-metalink-xml-prefix-trust.py | 194
> > +++++++++++++++++++++++++
> > testenv/Test-metalink-xml-prefix.py | 194
> > +++++++++++++++++++++++++
> > testenv/Test-metalink-xml-relpath-trust.py | 193
> > +++++++++++++++++++++++++
> > testenv/Test-metalink-xml-relpath.py | 126 ++++++++++++----
> > testenv/Test-metalink-xml-relprefix-trust.py | 194
> > +++++++++++++++++++++++++
> > testenv/Test-metalink-xml-relprefix.py | 194
> > +++++++++++++++++++++++++
> > testenv/Test-metalink-xml-trust.py | 197
> > ++++++++++++++++++++++++++
> > testenv/Test-metalink-xml.py | 126 ++++++++++++++--
> > 20 files changed, 2803 insertions(+), 84 deletions(-)
> > create mode 100755 testenv/Test-metalink-xml-abspath-trust.py
> > create mode 100755 testenv/Test-metalink-xml-absprefix-trust.py
> > create mode 100755 testenv/Test-metalink-xml-absprefix.py
> > create mode 100755 testenv/Test-metalink-xml-homepath-trust.py
> > create mode 100755 testenv/Test-metalink-xml-homeprefix-trust.py
> > create mode 100755 testenv/Test-metalink-xml-homeprefix.py
> > create mode 100755 testenv/Test-metalink-xml-prefix-trust.py
> > create mode 100755 testenv/Test-metalink-xml-prefix.py
> > create mode 100755 testenv/Test-metalink-xml-relpath-trust.py
> > create mode 100755 testenv/Test-metalink-xml-relprefix-trust.py
> > create mode 100755 testenv/Test-metalink-xml-relprefix.py
> > create mode 100755 testenv/Test-metalink-xml-trust.py
> >
> > diff --git a/doc/metalink-standard.txt b/doc/metalink-standard.txt
> > index d00c384..18acaaa 100644
> > --- a/doc/metalink-standard.txt
> > +++ b/doc/metalink-standard.txt
> > @@ -29,18 +29,61 @@ paths or descend/escalate to a relative path
> > unexpectedly.
> > 2.1 Metalink/XML implemented tests
> > ==================================
> >
> > -* testenv/Test-metalink-xml.py: Accept safe paths
> > -* testenv/Test-metalink-xml-abspath.py: Reject absolute paths
> > -* testenv/Test-metalink-xml-relpath.py: Reject relative paths
> > -* testenv/Test-metalink-xml-homepath.py: Reject home paths
> > +See testenv/Makefile.am (METALINK_TESTS).
> > +
> > +2.2 Metalink/HTTP implemented tests
> > +===================================
> > +
> > +See testenv/Makefile.am (METALINK_TESTS).
> >
> > 3. Download file name
> > *********************
> >
> > -Computing the file name to wrote from the followed urls only leads to
> > -uncertainty. Reason why an unique name shall be used. Respectively, it
> > -shall be the metalink:file "name" field for Metalink/XML and a derived
> > -cli's url for Metalink/HTTP.
> > +The download file name shall be decided by precise rules which prevent
> > +any naming uncertainty and security issues.
> > +
> > +3.1 Naming rules
> > +================
> > +
> > +The final name of downloaded files is computed starting from a trusted
> > +name, which is then combined with the "Directory Options". The result
> > +is verified and eventually made safer following security rules. If the
> > +final name isn't found safe enough, then the file isn't downloaded.
> > +
> > +Depending on the options used, a suffix could be appended to the final
> > +name to not overwrite existing files.
> > +
> > +3.1.1 The trusted name
> > +======================
> > +
> > +The option --trust-server-names decides what is the trusted name.
> > +
> > +Any Metalink/XML element with an unsafe metalink:file "name" field is
> > +ignored, see '1. Security features'.
> > +
> > +3.1.1.1 Without --trust-server-names
> > +====================================
> > +
> > +When --trust-server-names is off, the basename of the --input-metalink
> > +file, if available, or of the mother URL is trusted.
> > +
> > +The files described by a Metalink/XML file will be named sequentially
> > +applying a suffix to the trusted name.
> > +
> > +3.1.1.2 With --trust-server-names
> > +=================================
> > +
> > +When --trust-server-names is on, the metalink:file "name" field parsed
> > +from Metalink/XML files is trusted. When no Metalink/XML is available,
> > +the mother URL is trusted.
> > +
> > +3.1.2 The final name
> > +====================
> > +
> > +The "Directory Options" are combined with the trusted name. The result
> > +is evaluated again by the '1. Security features'. If the path is found
> > +unsafe, only the basename of the final name is considered. If this is
> > +found unsafe too, the file is not downloaded.
> >
> > 4. Metalink/XML
> > ***************
> > diff --git a/src/metalink.c b/src/metalink.c
> > index a8b047b..356a68b 100644
> > --- a/src/metalink.c
> > +++ b/src/metalink.c
> > @@ -68,7 +68,16 @@ retrieve_from_metalink (const metalink_t* metalink)
> > bool _output_stream_regular = output_stream_regular;
> > char *_output_document = opt.output_document;
> >
> > - DEBUGP (("Retrieving from Metalink\n"));
> > + /* metalink file counter */
> > + unsigned mfc = 0;
> > +
> > + /* metalink retrieval type */
> > + const char *metatpy = metalink->origin ? "Metalink/HTTP" :
> > "Metalink/XML";
> > +
> > + /* metalink mother source */
> > + char *metasrc = metalink->origin ? metalink->origin : opt.input_metalink;
> > +
> > + DEBUGP (("Retrieving from Metalink %s\n", quote (metasrc)));
> >
> > /* No files to download. */
> > if (!metalink->files)
> > @@ -86,6 +95,8 @@ retrieve_from_metalink (const metalink_t* metalink)
> > {
> > metalink_file_t *mfile = *mfile_ptr;
> > metalink_resource_t **mres_ptr;
> > + char *planname = NULL;
>
> could this be named plannedname?
planname goes well with filename, basename, safename, etc.
Isn't planname intuitive enough in such context?
>
> > @@ -103,30 +114,76 @@ retrieve_from_metalink (const metalink_t* metalink)
> >
> > output_stream = NULL;
> >
> > + mfc++;
> > +
> > /* The directory prefix for opt.metalink_over_http is handled by
> > src/url.c (url_file_name), do not add it a second time. */
> > if (!metalink->origin && opt.dir_prefix && strlen (opt.dir_prefix))
> > - filename = aprintf("%s/%s", opt.dir_prefix, mfile->name);
> > + planname = aprintf("%s/%s", opt.dir_prefix, mfile->name);
>
> missing space after the function name, though it will be probably fixed
> with the rebase on the previous change.
Fixed. Posting after final decisions are taken about open topics in this series
of patches.
>
> Giuseppe
Regards,
Matthew
--
Matthew White <address@hidden>
pgpUKMkcMDvJG.pgp
Description: PGP signature