Record a new solve before calculating the new dynamic value. (#919)

* Record a new solve before calculating the new dynamic value so hidden solves can't unintentionally move the challenge value

CTFd/plugins/dynamic_challenges/__init__.py

@@ -171,11 +171,23 @@ class DynamicValueChallenge(BaseChallenge):
 
         Model = get_model()
 
+        solve = Solves(
+            user_id=user.id,
+            team_id=team.id if team else None,
+            challenge_id=challenge.id,
+            ip=get_ip(req=request),
+            provided=submission
+        )
+        db.session.add(solve)
+
         solve_count = Solves.query \
             .join(Model, Solves.account_id == Model.id) \
             .filter(Solves.challenge_id == challenge.id, Model.hidden == False, Model.banned == False) \
             .count()
 
+        # We subtract -1 to allow the first solver to get max point value
+        solve_count -= 1
+
         # It is important that this calculation takes into account floats.
         # Hence this file uses from __future__ import division
         value = (
@@ -191,14 +203,6 @@ class DynamicValueChallenge(BaseChallenge):
 
         chal.value = value
 
-        solve = Solves(
-            user_id=user.id,
-            team_id=team.id if team else None,
-            challenge_id=challenge.id,
-            ip=get_ip(req=request),
-            provided=submission
-        )
-        db.session.add(solve)
         db.session.commit()
         db.session.close()
 

tests/challenges/test_dynamic.py

@@ -226,3 +226,67 @@ def test_dynamic_challenge_loses_value_properly():
                     assert chal.initial >= chal.value
                     assert chal.value > chal.minimum
     destroy_ctfd(app)
+
+
+def test_dynamic_challenge_value_isnt_affected_by_hidden_users():
+    app = create_ctfd(enable_plugins=True)
+    with app.app_context():
+        register_user(app)
+        client = login_as_user(app, name="admin", password="password")
+
+        challenge_data = {
+            "name": "name",
+            "category": "category",
+            "description": "description",
+            "value": 100,
+            "decay": 20,
+            "minimum": 1,
+            "state": "visible",
+            "type": "dynamic"
+        }
+
+        r = client.post('/api/v1/challenges', json=challenge_data)
+        assert r.get_json().get('data')['id'] == 1
+
+        gen_flag(app.db, challenge_id=1, content='flag')
+
+        # Make a solve as a regular user. This should not affect the value.
+        data = {
+            "submission": 'flag',
+            "challenge_id": 1
+        }
+
+        r = client.post('/api/v1/challenges/attempt', json=data)
+        resp = r.get_json()['data']
+        assert resp['status'] == 'correct'
+
+        # Make solves as hidden users. Also should not affect value
+        for i, team_id in enumerate(range(2, 26)):
+            name = "user{}".format(team_id)
+            email = "user{}@ctfd.io".format(team_id)
+            # We need to bypass rate-limiting so gen_user instead of register_user
+            user = gen_user(app.db, name=name, email=email)
+            user.hidden = True
+            app.db.session.commit()
+
+            with app.test_client() as client:
+                # We need to bypass rate-limiting so creating a fake user instead of logging in
+                with client.session_transaction() as sess:
+                    sess['id'] = team_id
+                    sess['name'] = name
+                    sess['type'] = 'user'
+                    sess['email'] = email
+                    sess['nonce'] = 'fake-nonce'
+
+                data = {
+                    "submission": 'flag',
+                    "challenge_id": 1
+                }
+
+                r = client.post('/api/v1/challenges/attempt', json=data)
+                resp = r.get_json()['data']
+                assert resp['status'] == 'correct'
+
+                chal = DynamicChallenge.query.filter_by(id=1).first()
+                assert chal.value == chal.initial
+    destroy_ctfd(app)