Block NULL characters in all POST methods (#1890)

funnel/models/account.py

@@ -113,9 +113,14 @@ class PROFILE_STATE(LabeledEnum):  # noqa: N801
 class ZBase32Comparator(Comparator[str]):  # pylint: disable=abstract-method
     """Comparator to allow lookup by Account.uuid_zbase32."""
 
-    def __eq__(self, other: str) -> sa.ColumnElement[bool]:  # type: ignore[override]
+    def __eq__(self, other: object) -> sa.ColumnElement[bool]:  # type: ignore[override]
         """Return an expression for column == other."""
-        return self.__clause_element__() == UUID(bytes=zbase32_decode(other))
+        try:
+            return self.__clause_element__() == UUID(  # type: ignore[return-value]
+                bytes=zbase32_decode(str(other))
+            )
+        except ValueError:  # zbase32 call failed, so it's not a valid string
+            return sa.false()
 
 
 class Account(UuidMixin, BaseMixin, Model):

funnel/views/api/oauth.py

@@ -25,7 +25,7 @@
 )
 from ...registry import resource_registry
 from ...typing import ReturnView
-from ...utils import abort_null, make_redirect_url
+from ...utils import make_redirect_url
 from ..login_session import reload_for_cookies, requires_client_login, requires_login
 from .resource import get_userinfo
 
@@ -415,20 +415,20 @@ def oauth_token_success(token: AuthToken, **params) -> ReturnView:
 def oauth_token() -> ReturnView:
     """Provide token endpoint for OAuth2 server."""
     # Always required parameters
-    grant_type = cast(Optional[str], abort_null(request.form.get('grant_type')))
+    grant_type = cast(Optional[str], request.form.get('grant_type'))
     auth_client = current_auth.auth_client  # Provided by @requires_client_login
-    scope = abort_null(request.form.get('scope', '')).split(' ')
+    scope = request.form.get('scope', '').split(' ')
     # if grant_type == 'authorization_code' (POST)
-    code = cast(Optional[str], abort_null(request.form.get('code')))
-    redirect_uri = cast(Optional[str], abort_null(request.form.get('redirect_uri')))
+    code = cast(Optional[str], request.form.get('code'))
+    redirect_uri = cast(Optional[str], request.form.get('redirect_uri'))
     # if grant_type == 'password' (POST)
-    username = cast(Optional[str], abort_null(request.form.get('username')))
-    password = cast(Optional[str], abort_null(request.form.get('password')))
+    username = cast(Optional[str], request.form.get('username'))
+    password = cast(Optional[str], request.form.get('password'))
     # if grant_type == 'client_credentials'
     buid = cast(
         Optional[str],
         # XXX: Deprecated userid parameter
-        abort_null(request.form.get('buid') or request.form.get('userid')),
+        request.form.get('buid') or request.form.get('userid'),
     )
 
     # Validations 1: Required parameters

funnel/views/api/shortlink.py

@@ -10,13 +10,12 @@
 
 from ... import app, shortlinkapp
 from ...models import Shortlink, db
-from ...utils import abort_null
 from ..helpers import app_url_for, validate_is_app_url
 
 
 # Add future hasjobapp route here
 @app.route('/api/1/shortlink/create', methods=['POST'])
-@requestform(('url', abort_null), ('shorter', getbool), ('name', abort_null))
+@requestform('url', ('shorter', getbool), 'name')
 def create_shortlink(
     url: str | furl, shorter: bool = True, name: str | None = None
 ) -> tuple[dict[str, str], int]:

funnel/views/api/sms_events.py

@@ -19,7 +19,6 @@
 )
 from ...transports.sms import validate_exotel_token
 from ...typing import ReturnView
-from ...utils import abort_null
 
 
 @app.route('/api/1/sms/twilio_event', methods=['POST'])
@@ -116,7 +115,7 @@ def process_exotel_event(secret_token: str) -> ReturnView:
     # If there are too many rejects, then most likely a hack attempt.
     statsd.incr('phone_number.event', tags={'engine': 'exotel', 'stage': 'received'})
 
-    exotel_to = abort_null(request.form.get('To', ''))
+    exotel_to = request.form.get('To', '')
     if not exotel_to:
         return {'status': 'eror', 'error': 'invalid_phone'}, 422
     # Exotel sends back 0-prefixed phone numbers, not plus-prefixed intl. numbers

funnel/views/contact.py

@@ -17,7 +17,7 @@
 from .. import app
 from ..models import ContactExchange, Project, TicketParticipant, db, sa
 from ..typing import ReturnRenderWith, ReturnView
-from ..utils import abort_null, format_twitter_handle
+from ..utils import format_twitter_handle
 from .login_session import requires_login
 
 
@@ -141,7 +141,7 @@ def scan(self) -> ReturnView:
 
     @route('scan/connect', endpoint='scan_connect', methods=['POST'])
     @requires_login
-    @requestargs(('puk', abort_null), ('key', abort_null))
+    @requestargs('puk', 'key')
     def connect(self, puk: str, key: str) -> ReturnView:
         """Verify a badge scan and create a contact."""
         ticket_participant = TicketParticipant.query.filter_by(puk=puk, key=key).first()

funnel/views/helpers.py

@@ -580,6 +580,16 @@ def shortlink(url: str, actor: Account | None = None, shorter: bool = True) -> s
 # --- Request/response handlers --------------------------------------------------------
 
 
+@app.before_request
+def no_null_in_form():
+    """Disallow NULL characters in any form submit (but don't scan file attachments)."""
+    if request.method == 'POST':
+        for values in request.form.listvalues():
+            for each in values:
+                if each is not None and '\x00' in each:
+                    abort(400)
+
+
 @app.after_request
 def commit_db_session(response: ResponseType) -> ResponseType:
     """Commit database session at the end of a request if asked to."""

funnel/views/label.py

@@ -13,7 +13,6 @@
 from ..forms import LabelForm, LabelOptionForm
 from ..models import Account, Label, Project, db
 from ..typing import ReturnRenderWith, ReturnView
-from ..utils import abort_null
 from .helpers import render_redirect
 from .login_session import requires_login, requires_sudo
 from .mixins import AccountCheckMixin, ProjectViewMixin
@@ -29,7 +28,7 @@ class ProjectLabelView(ProjectViewMixin, UrlForView, ModelView):
     def labels(self) -> ReturnRenderWith:
         form = forms.Form()
         if form.validate_on_submit():
-            namelist = [abort_null(x) for x in request.values.getlist('name')]
+            namelist = request.values.getlist('name')
             for idx, lname in enumerate(namelist, start=1):
                 lbl = Label.query.filter_by(project=self.obj, name=lname).first()
                 if lbl is not None:
@@ -51,8 +50,8 @@ def new_label(self) -> ReturnRenderWith:
             # and those values are also available at `form.data`.
             # But in case there are options, the option values are in the list
             # in the order they appeared on the create form.
-            titlelist = [abort_null(x) for x in request.values.getlist('title')]
-            emojilist = [abort_null(x) for x in request.values.getlist('icon_emoji')]
+            titlelist = request.values.getlist('title')
+            emojilist = request.values.getlist('icon_emoji')
             # first values of both lists belong to the parent label
             titlelist.pop(0)
             emojilist.pop(0)
@@ -143,9 +142,9 @@ def edit(self) -> ReturnRenderWith:
             return render_redirect(self.obj.project.url_for('labels'))
 
         if form.validate_on_submit():
-            namelist = [abort_null(x) for x in request.values.getlist('name')]
-            titlelist = [abort_null(x) for x in request.values.getlist('title')]
-            emojilist = [abort_null(x) for x in request.values.getlist('icon_emoji')]
+            namelist = request.values.getlist('name')
+            titlelist = request.values.getlist('title')
+            emojilist = request.values.getlist('icon_emoji')
 
             namelist.pop(0)
             titlelist.pop(0)

funnel/views/login.py

@@ -153,7 +153,7 @@ def login() -> ReturnView:
     if request.method == 'GET':
         loginmethod = request.cookies.get('login')
 
-    formid = abort_null(request.form.get('form.id'))
+    formid = request.form.get('form.id')
     if request.method == 'POST' and formid == 'passwordlogin':
         try:
             success = loginform.validate()

funnel/views/login_session.py

@@ -646,7 +646,7 @@ def wrapper(*args: P.args, **kwargs: P.kwargs) -> T | ReturnResponse:
                 return render_redirect(url_for('change_password'))
 
         elif request.method == 'POST':
-            formid = abort_null(request.form.get('form.id'))
+            formid = request.form.get('form.id')
             if formid == FORMID_SUDO_OTP:
                 try:
                     otp_session = OtpSession.retrieve('sudo')

funnel/views/siteadmin.py

@@ -453,8 +453,8 @@ def init_rq_dashboard():
     """Register RQ Dashboard Blueprint if available for import."""
     if rq_dashboard is not None:
         rq_dashboard.blueprint.before_request(
-            lambda: None
-            if current_auth and current_auth.user.is_sysadmin
-            else abort(403)
+            lambda: (
+                None if current_auth and current_auth.user.is_sysadmin else abort(403)
+            )
         )
         app.register_blueprint(rq_dashboard.blueprint, url_prefix='/siteadmin/rq')

funnel/views/ticket_participant.py

@@ -33,13 +33,7 @@
 from ..proxies import request_wants
 from ..signals import user_data_changed, user_registered
 from ..typing import ReturnRenderWith, ReturnView
-from ..utils import (
-    abort_null,
-    format_twitter_handle,
-    make_qrcode,
-    mask_email,
-    split_name,
-)
+from ..utils import format_twitter_handle, make_qrcode, mask_email, split_name
 from .helpers import render_redirect
 from .login_session import requires_login
 from .mixins import AccountCheckMixin, ProjectViewMixin, TicketEventViewMixin
@@ -271,9 +265,7 @@ def checkin(self) -> ReturnView:
         form = forms.Form()
         if form.validate_on_submit():
             checked_in = getbool(request.form.get('checkin'))
-            ticket_participant_ids = [
-                abort_null(x) for x in request.form.getlist('puuid_b58')
-            ]
+            ticket_participant_ids = request.form.getlist('puuid_b58')
             for ticket_participant_id in ticket_participant_ids:
                 attendee = TicketEventParticipant.get(self.obj, ticket_participant_id)
                 attendee.checked_in = checked_in