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

H00die/flask unsign #24

Merged
merged 4 commits into from
Sep 9, 2023
Merged

Conversation

zeroSteiner
Copy link

This makes some changes. I'll leave a review explaining the important ones.

Copy link
Author

@zeroSteiner zeroSteiner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested this with:

  • Apache Superset
  • A custom Flask app I run
  • A stock Flask-app using the quick start

While testing these three instances I found a few issues and opportunities for improvements which I have implemented herein.

app.py Most of the boiler plat was taken from the [Quickstart Guide.](https://flask.palletsprojects.com/en/2.3.x/quickstart/#sessions) Changes were made to ensure that `/` always created a session and included an additional cookie.
from flask import Flask, session, redirect, url_for, request, make_response
from markupsafe import escape

import random, string

app = Flask(__name__)

# Set the secret key to some random bytes. Keep this really secret!
app.secret_key = b'_5#y2L"F4Q8z\n\xec]/'
#app.secret_key = b'CHANGE_ME_TO_A_COMPLEX_RANDOM_SECRET'

@app.route('/')
def index():
    if not 'csrf_token' in session:
        session['csrf_token'] = ''.join(random.choices(string.ascii_lowercase, k=10))
    if 'username' in session:
        resp = make_response('Logged in as %s' % escape(session['username']))
    else:
        resp = make_response('You are not logged in')
    if not request.cookies.get('CSRF'):
        resp.set_cookie('CSRF', session['csrf_token'])
    return resp

@app.route('/login', methods=['GET', 'POST'])
def login():
    if request.method == 'POST':
        session['username'] = request.form['username']
        return redirect(url_for('index'))
    return '''
        <form method="post">
            <p><input type=text name=username>
            <p><input type=submit value=Login>
        </form>
    '''

@app.route('/logout')
def logout():
    # remove the username from the session if it's there
    session.pop('username', None)
    return redirect(url_for('index'))

Retrieve, Alter and Resign a cookie
Resign the specified cookie data
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This action didn't actually retrieve it. It functioned entirely offline.

@@ -34,7 +34,7 @@ def get_signature(value)
end
end

class TimestampSigner < Signer
class URLSafeTimedSigner < URLSafeSigner
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed this name so if we need to add another signer in the future, we'll be in a better spot to do so before it's used all over the place.

@@ -170,7 +171,7 @@ def run
status: Metasploit::Model::Login::Status::UNTRIED
})
end
print_good("Found #{db_type} database #{db_name}: #{db_user}:#{db_pass}@#{db_host}:#{db_port}")
print_good("Found #{db_display_name}: #{db_type}://#{db_user}:#{db_pass}@#{db_host}:#{db_port}/#{db_name}")
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This adds the display name to the output which might be handy and formats the rest as a connection string.

Comment on lines +52 to +55
register_advanced_options(
[
OptString.new('CookieName', [ true, 'The name of the session cookie', 'session' ]),
OptString.new('Salt', [ true, 'The salt to use for key derivation', Msf::Exploit::Remote::HTTP::FlaskUnsign::Session::DEFAULT_SALT ])
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These advanced options grant more flexibility to the user so if non-default values are in use, the module will still work. Some Flask extensions such as Flask-Session use a non-default salt.

Comment on lines +69 to +70
cookie = cookie_jar.cookies.find { |c| c.name == datastore['CookieName'] }&.cookie_value
fail_with(Failure::UnexpectedReply, "#{peer} - Response is missing the session cookie") unless cookie
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was necessary to handle instances where the server returned more than one cookie.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I moved this file to follow the convention that what is being tested is the Session module which is a child of FlaskUnsign. If the modules are broken into new files, this one is already where it belongs.

Comment on lines +31 to +39
describe '.valid?' do
it 'verifies a signed cookie' do
expect(Msf::Exploit::Remote::HTTP::FlaskUnsign::Session.valid?('eyJoZWxsbyI6IndvcmxkIn0.ZKvywA.s78heXzx4hJKO55wwu5X7RiS164', secret)).to be true
end

it 'does not verify an invalid signed cookie' do
expect(Msf::Exploit::Remote::HTTP::FlaskUnsign::Session.valid?('eyJoZWxsbyI6IndvcmxkIn0.ZKvywA.s78heXzx4hJKO55wwu5X7RiS163', secret)).to be false
end
end
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

New tests as requested 😄

@h00die h00die merged commit 15c6f0d into h00die:flask_unsign Sep 9, 2023
27 of 28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants