#13 Security hole in exec cURL

closed-fixed
Gene Wood
None
5
2014-07-22
2002-10-22
Tajh Leitso
No

The exec statement that wraps the curl command needs to have
its parameters sanity checked. Also included is a form submit
redirect patch.

my patch:
311c311,314
< $this->submit($this->
_redirectaddr,$formvars, $formfiles);
---
> if( strpos( $this->
_redirectaddr, "?" ) > 0 )
> $this->
fetch($this->_redirectaddr); // the redirect has turned the request
into the get method
> else
> $this->
submit($this->_redirectaddr,$formvars, $formfiles);
372c375,378
< $this->submit($this->
_redirectaddr,$formvars, $formfiles);
---
> if( strpos( $this->
_redirectaddr, "?" ) > 0 )
> $this->fetch($this->
_redirectaddr); // the redirect has turned the request into the get
method
> else
> $this->submit($this->
_redirectaddr,$formvars, $formfiles);
929c935,936
< exec($this->curl_path." -D \"/tmp/$headerfile\ "".$cmdline_params." ".$URI,$results,$return);
---
> $safer_URI = strtr( $URI, "\"", " " ); // strip quotes from
the URI to avoid shell access
> exec($this->curl_path." -D \"/tmp/$headerfile\ "".$cmdline_params." \"".$safer_URI."\"",$results,$return);

Or Context:
*** Snoopy-1.0/Snoopy.class.Inc Wed Oct 9 14:13:46 2002
--- Snoopy-1.0.mod/Snoopy.class.inc Tue Oct 22 05:53:01
2002
***************
*** 308,314 ****
/* follow the redirect */
$this->
_redirectdepth++;
$this->
lastredirectaddr=$this->_redirectaddr;
! $this->submit($this->
_redirectaddr,$formvars, $formfiles);
}
}
}
--- 308,317 ----
/* follow the redirect */
$this->
_redirectdepth++;
$this->
lastredirectaddr=$this->_redirectaddr;
! if( strpos( $this->
_redirectaddr, "?" ) > 0 )
! $this->
fetch($this->_redirectaddr); // the redirect has turned the request
into the get method
! else
! $this->
submit($this->_redirectaddr,$formvars, $formfiles);
}
}
}
***************
*** 369,375 ****
/* follow the redirect */
$this->_redirectdepth++;
$this->lastredirectaddr=
$this->_redirectaddr;
! $this->submit($this->
_redirectaddr,$formvars, $formfiles);
}
}
}
--- 372,381 ----
/* follow the redirect */
$this->_redirectdepth++;
$this->lastredirectaddr=
$this->_redirectaddr;
! if( strpos( $this->
_redirectaddr, "?" ) > 0 )
! $this->fetch($this->
_redirectaddr); // the redirect has turned the request into the get
method
! else
! $this->submit($this->
_redirectaddr,$formvars, $formfiles);
}
}
}
***************
*** 926,932 ****

$headerfile = uniqid(time());

! exec($this->curl_path." -D \"/tmp/$headerfile\ "".$cmdline_params." ".$URI,$results,$return);

if($return)
{
--- 932,939 ----

$headerfile = uniqid(time());

! $safer_URI = strtr( $URI, "\"", " " ); // strip quotes from
the URI to avoid shell access
! exec($this->curl_path." -D \"/tmp/$headerfile\ "".$cmdline_params." \"".$safer_URI."\"",$results,$return);

if($return)
{

Discussion

  • Tajh Leitso
    Tajh Leitso
    2002-10-22

    My patch in a text file

     
    Attachments
  • Gene Wood
    Gene Wood
    2004-10-16

    Logged In: YES
    user_id=547273

    Both of your fixes have been checked into CVS. Thanks for
    the patch. Good eye on the security risk in the exec statement.

     
  • Gene Wood
    Gene Wood
    2004-10-16

    • assigned_to: nobody --> gene_wood
    • status: open --> closed-fixed