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

Strings with NUL bytes are silently truncated in bound parameters #420

Closed
benkuhn opened this issue Mar 30, 2016 · 4 comments
Closed

Strings with NUL bytes are silently truncated in bound parameters #420

benkuhn opened this issue Mar 30, 2016 · 4 comments

Comments

@benkuhn
Copy link

benkuhn commented Mar 30, 2016

Minimal repro:

#!python
import psycopg2

conn = psycopg2.connect(host='localhost', dbname='nullbytes')
cursor = conn.cursor()

try:
    cursor.execute("""drop table "user" """)
except:
    pass

cursor.execute("""
CREATE TABLE "user" (
    email_address VARCHAR(60)
)
""")

cursor.execute(
    """INSERT INTO "user" (email_address) VALUES (%(email_address)s)""",
    {'email_address': 'ben.s.kuhn@gmail.com\x00 foo'})

cursor.execute("""
SELECT "user".email_address
FROM "user"
WHERE "user".email_address = %(email_address_1)s
""", {'email_address_1': 'ben.s.kuhn@gmail.com\x00 huh??'})


print(cursor.fetchall())

This prints out [('ben.s.kuhn@gmail.com',)], which is neither what I tried to insert nor what I tried to query for, which came as a surprise to me! I would have expected some kind of exception to be thrown. This has the potential to cause security flaws if a developer assumes that the string being queried/inserted has passed validation because the query worked.

@dvarrazzo
Copy link
Member

I'll look into that. However you may know that postgres doesn't accept null bytes in strings, so the right thing to do would be to raise an exception, right?

@benkuhn
Copy link
Author

benkuhn commented Mar 30, 2016

Yes, raising an exception would be awesome :) Thanks!

On Wed, Mar 30, 2016 at 2:59 PM, Daniele Varrazzo notifications@github.com
wrote:

I'll look into that. However you may know that postgres doesn't accept
null bytes in strings, so the right thing to do would be to raise an
exception, right?


You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
#420 (comment)

@flupzor
Copy link
Contributor

flupzor commented Jul 17, 2016

I've fixed this issue here:

#459

@benkuhn
Copy link
Author

benkuhn commented Jul 18, 2016

Amazing, thanks so much!

dvarrazzo added a commit that referenced this issue Aug 7, 2016
dvarrazzo added a commit that referenced this issue Dec 1, 2017
- 2.6.3 has not been released (yet). Fixes for bug #420, bug #462 were
  relased in 2.7.
- Added missing report for bug #489 fixed in 2.7.
dvarrazzo added a commit that referenced this issue Dec 1, 2017
- 2.6.3 has not been released (yet). Fixes for bug #420, bug #462 were
  relased in 2.7.
- Added missing report for bug #489 fixed in 2.7.
acrewdson added a commit to acrewdson/wagtail that referenced this issue Jun 26, 2018
After migrating a Wagtail-based site from MySQL to Postgres, we
noticed that malicious requests to the site that included percent-
encoded Unicode NULLs (`%00`) raised a `ValueError` exception that we
hadn't seen when using MySQL: `A string literal cannot contain NUL
(0x00) characters.` This appears to relate to `psycopg2`'s decision to
raise an exception in these situations, as discussed here:

    psycopg/psycopg2#420

While newer versions of Django appear to provide some field validation
that addresses these characters, it doesn't look like Wagtail's
redirect middleware is making use of those validators, and so it seemed
reasonable to clean these characters in the context of 'normalizing'
the paths before looking for corresponding redirects -- especially
since a quick investigation on the internet suggests that U+0000 in
URLs can be used as a means of attack, and also since RFC 3986 says:

   Note, however, that the "%00" percent-encoding (NUL) may require
   special handling and should be rejected if the application is not
   expecting to receive raw data within a component.
chosak pushed a commit to wagtail/wagtail that referenced this issue Jun 27, 2018
After migrating a Wagtail-based site from MySQL to Postgres, we
noticed that malicious requests to the site that included percent-
encoded Unicode NULLs (`%00`) raised a `ValueError` exception that we
hadn't seen when using MySQL: `A string literal cannot contain NUL
(0x00) characters.` This appears to relate to `psycopg2`'s decision to
raise an exception in these situations, as discussed here:

    psycopg/psycopg2#420

While newer versions of Django appear to provide some field validation
that addresses these characters, it doesn't look like Wagtail's
redirect middleware is making use of those validators, and so it seemed
reasonable to clean these characters in the context of 'normalizing'
the paths before looking for corresponding redirects -- especially
since a quick investigation on the internet suggests that U+0000 in
URLs can be used as a means of attack, and also since RFC 3986 says:

   Note, however, that the "%00" percent-encoding (NUL) may require
   special handling and should be rejected if the application is not
   expecting to receive raw data within a component.
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

No branches or pull requests

3 participants