Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Have attacker rerun all requests, update xfail test #44

Merged
merged 8 commits into from
Apr 10, 2020
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions fuzz_lightyear/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
from .output.interface import ResultFormatter
from .output.logging import log
from .output.util import print_error
from .runner import run_sequence
from .runner import validate_sequence
from .settings import get_settings
from .supplements.abstraction import get_abstraction
from .usage import parse_args
Expand Down Expand Up @@ -119,7 +119,7 @@ def run_tests(
tests=tests,
):
try:
run_sequence(result.requests, result.responses)
validate_sequence(result.requests, result.responses)
except Exception as e:
if (
ignore_exceptions and
Expand Down
12 changes: 9 additions & 3 deletions fuzz_lightyear/plugins/idor.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@

from ..datastore import get_non_vulnerable_operations
from ..request import FuzzingRequest
from ..response import ResponseSequence
from ..runner import run_sequence
from ..supplements.abstraction import get_abstraction
from .base import BasePlugin

Expand Down Expand Up @@ -34,13 +36,17 @@ def is_vulnerable(
request_sequence: List[FuzzingRequest],
response_sequence: List[Any],
) -> bool:
last_request = request_sequence[-1]
run_sequence(
sequence=request_sequence[:-1],
responses=ResponseSequence(),
auth=get_abstraction().get_attacker_session(), # type: ignore
)
try:
last_request.send(
request_sequence[-1].send(
auth=get_abstraction().get_attacker_session(), # type: ignore
should_log=False,
)

return True

except (HTTPError, SwaggerMappingError, ValidationError):
return False
3 changes: 2 additions & 1 deletion fuzz_lightyear/response.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
from typing import Dict
from typing import List

from .plugins import get_enabled_plugins
from .request import FuzzingRequest


Expand Down Expand Up @@ -33,6 +32,8 @@ def analyze_requests(
self,
request_sequence: List[FuzzingRequest],
) -> None:
# We import here because otherwise we run into a circular dependency
from .plugins import get_enabled_plugins
plugins = get_enabled_plugins()
for plugin in plugins:
if plugin.should_run(
Expand Down
22 changes: 18 additions & 4 deletions fuzz_lightyear/runner.py
Original file line number Diff line number Diff line change
@@ -1,19 +1,37 @@
from typing import Any
from typing import Dict
from typing import List
from typing import Optional

from .datastore import clear_cache
from .datastore import get_user_defined_mapping
from .request import FuzzingRequest
from .response import ResponseSequence


def validate_sequence(
sequence: List[FuzzingRequest],
responses: ResponseSequence,
) -> ResponseSequence:

run_sequence(sequence, responses)
# Then, check for vulnerabilities.
responses.analyze_requests(sequence)
clear_cache()
return responses


def run_sequence(
sequence: List[FuzzingRequest],
responses: ResponseSequence,
auth: Optional[Dict[str, Any]] = None,
) -> ResponseSequence:

# First, determine whether this is a successful request sequence.
for request in sequence:
response = request.send(
data=responses.data,
auth=auth,
)

responses.add_response(response)
Expand All @@ -26,8 +44,4 @@ def run_sequence(
for key in request.fuzzed_input: # type: ignore
if key in get_user_defined_mapping():
responses.data[key] = request.fuzzed_input[key] # type: ignore

# Then, check for vulnerabilities.
responses.analyze_requests(sequence)
clear_cache()
return responses
2 changes: 1 addition & 1 deletion requirements-dev.txt
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
aniso8601==7.0.0
aspy.yaml==1.3.0
atomicwrites==1.3.0
attrs==19.1.0
attrs==19.2.0
bravado==10.4.1
bravado-core==5.13.1
cached-property==1.5.1
Expand Down
2 changes: 1 addition & 1 deletion requirements.txt
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
attrs==19.1.0
attrs==19.2.0
bravado==10.4.1
bravado-core==5.13.1
cached-property==1.5.1
Expand Down
7 changes: 7 additions & 0 deletions testing/vulnerable_app/views/models/database.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,3 +20,10 @@
'id': fields.Integer(required=True),
},
)

thing_model = api.model(
'Thing',
{
'id': fields.Integer(required=True),
},
)
tanx16 marked this conversation as resolved.
Show resolved Hide resolved
34 changes: 30 additions & 4 deletions testing/vulnerable_app/views/sequence.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,6 @@ class CreateWithSideEffect(Resource):
@api.response(200, 'Success', model=widget_model)
@requires_user
def post(self, user):
user.has_created_resource = True
user.save()

with database.connection() as session:
entry = Widget()
Expand All @@ -78,19 +76,22 @@ def post(self, user):

widget_id = entry.id

user.created_resource = [entry.id]
user.save()

return {
'id': widget_id,
}


@ns.route('/side-effect/get/<int:id>')
class GetWithSideEffect(Resource):
class GetWithSideEffectUnsafe(Resource):
@api.doc(security='apikey')
@api.response(200, 'Success', model=widget_model)
@api.response(404, 'Not Found')
@requires_user
def get(self, id, user):
if not user.has_created_resource:
if not user.created_resource:
abort(401)

with database.connection() as session:
Expand All @@ -106,3 +107,28 @@ def get(self, id, user):
return {
'id': widget_id,
}


@ns.route('/side-effect-safe/get/<int:id>')
class GetWithSideEffectSafe(Resource):
@api.doc(security='apikey')
@api.response(200, 'Success', model=widget_model)
@api.response(404, 'Not Found')
@requires_user
def get(self, id, user):
if id not in user.created_resource:
abort(403)

with database.connection() as session:
try:
entry = session.query(Widget).filter(
Widget.id == id,
).one()
except NoResultFound:
abort(404)

widget_id = entry.id

return {
'id': widget_id,
}
44 changes: 32 additions & 12 deletions tests/integration/plugins/idor_test.py
Original file line number Diff line number Diff line change
@@ -1,12 +1,10 @@
import pytest

from fuzz_lightyear.request import FuzzingRequest
from fuzz_lightyear.response import ResponseSequence
from fuzz_lightyear.runner import run_sequence
from fuzz_lightyear.runner import validate_sequence


def test_basic(mock_client):
responses = run_sequence(
responses = validate_sequence(
[
FuzzingRequest(
tag='basic',
Expand All @@ -22,7 +20,7 @@ def test_basic(mock_client):


def test_skipped_due_to_no_inputs(mock_client):
responses = run_sequence(
responses = validate_sequence(
[
FuzzingRequest(
tag='basic',
Expand All @@ -36,11 +34,8 @@ def test_skipped_due_to_no_inputs(mock_client):
assert responses.test_results == {}


@pytest.mark.xfail(
reason='https://github.com/Yelp/fuzz-lightyear/issues/11',
)
def test_side_effect(mock_api_client):
responses = run_sequence(
def test_side_effect_unsafe(mock_api_client):
responses = validate_sequence(
[
FuzzingRequest(
tag='sequence',
Expand All @@ -54,11 +49,36 @@ def test_side_effect(mock_api_client):
# This goes last, to test for IDOR.
FuzzingRequest(
tag='sequence',
operation_id='get_get_with_side_effect',
operation_id='get_get_with_side_effect_unsafe',
),
],
ResponseSequence(),
)

assert responses.responses[1].has_created_resource
assert responses.responses[1].created_resource
assert responses.test_results['IDORPlugin']


def test_side_effect_safe(mock_api_client):
responses = validate_sequence(
[
FuzzingRequest(
tag='sequence',
operation_id='post_create_with_side_effect',
),
FuzzingRequest(
tag='user',
operation_id='get_get_user',
),

# This goes last, to test for IDOR.
FuzzingRequest(
tag='sequence',
operation_id='get_get_with_side_effect_safe',
),
],
ResponseSequence(),
)

assert responses.responses[1].created_resource
assert not responses.test_results['IDORPlugin']
10 changes: 5 additions & 5 deletions tests/integration/runner_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
import fuzz_lightyear
from fuzz_lightyear.request import FuzzingRequest
from fuzz_lightyear.response import ResponseSequence
from fuzz_lightyear.runner import run_sequence
from fuzz_lightyear.runner import validate_sequence


@pytest.fixture
Expand All @@ -21,7 +21,7 @@ def get_exclusions():

def test_invalid_request(mock_client):
with pytest.raises(HTTPError):
run_sequence(
validate_sequence(
[
FuzzingRequest(
tag='constant',
Expand All @@ -45,7 +45,7 @@ def test_valid_request_skip_idor_manually_excluded(
mock_client,
non_vulnerable_operations,
):
responses = run_sequence(
responses = validate_sequence(
[
FuzzingRequest(
tag='basic',
Expand All @@ -62,7 +62,7 @@ def test_valid_request_skip_idor_manually_excluded(
class TestStatefulSequence:

def test_basic(self, mock_client):
responses = run_sequence(
responses = validate_sequence(
[
FuzzingRequest(
tag='sequence',
Expand Down Expand Up @@ -90,7 +90,7 @@ def create_resource():

return output
fuzz_lightyear.register_factory('id')(create_resource)
responses = run_sequence(
responses = validate_sequence(
[
FuzzingRequest(
tag='sequence',
Expand Down