bug-wget
[Top][All Lists]
Advanced

[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>

Attachment: pgpUKMkcMDvJG.pgp
Description: PGP signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]