Skip to content
This repository has been archived by the owner on Sep 5, 2018. It is now read-only.

Sandbox escape due to wrapper functions lacking container logic #18

Closed
ChaosData opened this issue Feb 22, 2015 · 1 comment
Closed

Comments

@ChaosData
Copy link

As mentioned in mrjoelkemp/phpepl#31, it appears that the wrap/&wrapByRef functions in functions.php currently handle container-like values such as arrays by punting on them. Generally this approach is fine as most things in arrays will be wrapped at some point by the sandbox, however, there are some cases where array-generating PHP library functions will sidestep the SecureString wrap hooking. In these cases, should the values of these unwrapped strings be passed as callable values to any of the callable functions (this SO post has a pretty good list, though I'm not sure how comprehensive it is given newer functions added in the past years).

As such, a few examples I could come up with are:

array_map('array_map', array('system'), array(array('cat /etc/hosts') ));
array_map('register_shutdown_function', array('system'), array('id'));

Note that in regards to the latter example, register_shutdown_function called directly with 'system' and 'id' would be caught by the sandbox even if register_shutdown_function itself were not directly.

Adding in extra clauses to the wrap/&wrapByRef functions to catch is_array() values would catch these issues, but I'm not sure if the same sorts of functions that generate unwrapped values within arrays could also be used to generate unwrapped values within other constructs (or if they would get wrapped properly anyway before they could ever touch anything dangerous).

Something like the below (admittedly a bit quick-and-dirty) would probably suffice, at least for the array case, to recursively wrap everything that could be contained in an array.

if ( is_array($value) ) {
  $a = array();
  foreach ($value as &$inner_value) {
    $a[] = wrap($inner_value, $sandbox);
  }
  return $a;
}
fieryprophet added a commit that referenced this issue Feb 27, 2015
…sing the sandbox by encapsulating unsandboxed callables
@fieryprophet
Copy link
Owner

Addressed in v.1.3.10, with some modification to preserve array keys and the current array pointer upon wrapping the array.

Thanks so much for reporting your findings and helping to improve the sandbox.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants