Commit 5ca96cb8 authored by Daniel Stenberg's avatar Daniel Stenberg
Browse files

urlglob: better detect unclosed braces, empty lists and overflows 

A rather big overhaul and cleanup.

1 - curl wouldn't properly detect and reject globbing that ended with an
open brace if there were brackets or braces before it. Like "{}{" or
"[0-1]{"

2 - curl wouldn't properly reject empty lists so that "{}{}" would
result in curl getting (nil) strings in the output.

3 - By using strtoul() instead of sscanf() the code will now detected
over and underflows. It now also better parses the step argument to only
accept positive numbers and only step counters that is smaller than the
delta between the maximum and minimum numbers.

4 - By switching to unsigned longs instead of signed ints for the
counters, the max values for []-ranges are now very large (on 64bit
machines).

5 - Bumped the maximum number of globs in a single URL to 100 (from 10)

6 - Simplified the code somewhat and now it stores fixed strings as
single- entry lists. That's also one of the reasons why I did (5) as now
all strings between "globs" will take a slot in the array.

Added test 1234 and 1235 to verify. Updated test 87.

This commit fixes three separate bug reports.

Bug: http://curl.haxx.se/bug/view.cgi?id=1264
Bug: http://curl.haxx.se/bug/view.cgi?id=1265
Bug: http://curl.haxx.se/bug/view.cgi?id=1266
Reported-by: Will Dietz
parent 10afe7cf

src/tool_operate.c

+8 −7
Changes for src/tool_operate.c: 8 added lines, 7 removed lines.
Original line number Diff line number Diff line
@@ -196,6 +196,7 @@ int operate(struct Configurable *config, int argc, argv_item_t argv[]) 
  bool stillflags;

  int res = 0;

  int i;

  unsigned long li;



  bool orig_noprogress;

  bool orig_isatty;
@@ -465,10 +466,10 @@ int operate(struct Configurable *config, int argc, argv_item_t argv[]) 


  for(urlnode = config->url_list; urlnode; urlnode = urlnode->next) {



    int up; /* upload file counter within a single upload glob */

    unsigned long up; /* upload file counter within a single upload glob */

    char *infiles; /* might be a glob pattern */

    char *outfiles;

    int infilenum;

    unsigned long infilenum;

    URLGlob *inglob;



    int metalink = 0; /* nonzero for metalink download. */
@@ -533,7 +534,7 @@ int operate(struct Configurable *config, int argc, argv_item_t argv[]) 
      char *uploadfile; /* a single file, never a glob */

      int separator;

      URLGlob *urls;

      int urlnum;

      unsigned long urlnum;



      uploadfile = NULL;

      urls = NULL;
@@ -583,7 +584,7 @@ int operate(struct Configurable *config, int argc, argv_item_t argv[]) 
      separator= ((!outfiles || curlx_strequal(outfiles, "-")) && urlnum > 1);



      /* Here's looping around each globbed URL */

      for(i = 0 ; i < urlnum; i++) {

      for(li = 0 ; li < urlnum; li++) {



        int infd;

        bool infdopen;
@@ -628,7 +629,7 @@ int operate(struct Configurable *config, int argc, argv_item_t argv[]) 
            if(res)

              goto show_error;

          }

          else if(!i) {

          else if(!li) {

            this_url = strdup(urlnode->url);

            if(!this_url) {

              res = CURLE_OUT_OF_MEMORY;
@@ -863,8 +864,8 @@ int operate(struct Configurable *config, int argc, argv_item_t argv[]) 
        }



        if(urlnum > 1 && !(config->mute)) {

          fprintf(config->errors, "\n[%d/%d]: %s --> %s\n",

                  i+1, urlnum, this_url, outfile ? outfile : "<stdout>");

          fprintf(config->errors, "\n[%lu/%lu]: %s --> %s\n",

                  li+1, urlnum, this_url, outfile ? outfile : "<stdout>");

          if(separator)

            printf("%s%s\n", CURLseparator, this_url);

        }

src/tool_urlglob.c

+192 −179

File changed.

Preview size limit exceeded, changes collapsed.

src/tool_urlglob.h

+13 −12
Changes for src/tool_urlglob.h: 13 added lines, 12 removed lines.
Original line number Diff line number Diff line
@@ -7,7 +7,7 @@ 
 *                            | (__| |_| |  _ <| |___

 *                             \___|\___/|_| \_\_____|

 *

 * Copyright (C) 1998 - 2012, Daniel Stenberg, <daniel@haxx.se>, et al.

 * Copyright (C) 1998 - 2013, Daniel Stenberg, <daniel@haxx.se>, et al.

 *

 * This software is licensed as described in the file COPYING, which

 * you should have received as part of this distribution. The terms
@@ -31,11 +31,13 @@ typedef enum { 


typedef struct {

  URLPatternType type;

  int globindex; /* the number of this particular glob or -1 if not used

                    within {} or [] */

  union {

    struct {

      char **elements;

      short size;

      short ptr_s;

      int size;

      int ptr_s;

    } Set;

    struct {

      char min_c;
@@ -44,21 +46,20 @@ typedef struct { 
      int step;

    } CharRange;

    struct {

      int min_n;

      int max_n;

      short padlength;

      int ptr_n;

      int step;

      unsigned long min_n;

      unsigned long max_n;

      int padlength;

      unsigned long ptr_n;

      unsigned long step;

    } NumRange ;

  } content;

} URLPattern;



/* the total number of globs supported */

#define GLOB_PATTERN_NUM 9

#define GLOB_PATTERN_NUM 100



typedef struct {

  char *literal[10];

  URLPattern pattern[GLOB_PATTERN_NUM+1];

  URLPattern pattern[GLOB_PATTERN_NUM];

  size_t size;

  size_t urllen;

  char *glob_buffer;
@@ -66,7 +67,7 @@ typedef struct { 
  char errormsg[80]; /* error message buffer */

} URLGlob;



int glob_url(URLGlob**, char*, int *, FILE *);

int glob_url(URLGlob**, char*, unsigned long *, FILE *);

int glob_next_url(char **, URLGlob *);

int glob_match_url(char **, char*, URLGlob *);

void glob_cleanup(URLGlob* glob);

tests/data/Makefile.am

+1 −1
Changes for tests/data/Makefile.am: 1 added line, 1 removed line.
Original line number Diff line number Diff line
@@ -93,7 +93,7 @@ test1200 test1201 test1202 test1203 test1204 test1205 test1206 test1207 \ 
test1208 test1209 test1210 test1211 test1212 test1213 test1214 test1215 \

test1216 test1217 test1218 test1219 \

test1220 test1221 test1222 test1223 test1224 test1225 test1226 test1227 \

test1228 test1229 test1230 test1231 test1232 test1233 \

test1228 test1229 test1230 test1231 test1232 test1233 test1234 test1235 \

\

test1300 test1301 test1302 test1303 test1304 test1305 test1306 test1307 \

test1308 test1309 test1310 test1311 test1312 test1313 test1314 test1315 \

tests/data/test1234

0 → 100644
+32 −0
Changes for tests/data/test1234: 32 added lines, 0 removed lines.
Original line number Diff line number Diff line 
<testcase>

<info>

<keywords>

{} list

FAILURE

</keywords>

</info>

# Server-side

<reply>

</reply>



# Client-side

<client>

<server>

none

</server>

 <name>

abusing {}-globbing

 </name>

 <command>

"%HOSTIP:%HTTPPORT/1234[0-1]{" "%HOSTIP:%HTTPPORT/{}{}{}{"

</command>

</client>



# Verify data after the test has been "shot"

<verify>

# 3 == CURLE_URL_MALFORMAT

<errorcode>

3

</errorcode>

</verify>

</testcase>