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

Disable SQL error output on production #1597

Closed
willdoran opened this issue Mar 6, 2017 · 12 comments
Closed

Disable SQL error output on production #1597

willdoran opened this issue Mar 6, 2017 · 12 comments
Assignees
Labels
Codebase: API Indicates issue work will be in API Tech Debt Theme: Security

Comments

@willdoran
Copy link
Contributor

willdoran commented Mar 6, 2017

Expected behaviour

  • Properly escape and filter user input. Only use prepared statements whencommunicating with a SQL database. Use additional escaping on the backend if thisis not already present. Don't return any SQL errors to the client.

Actual behaviour

  • Sql errors appeared on several pages. It was possible to injectSQL commands but getting a working query was not possible.

Steps to reproduce the behaviour/error

@willdoran willdoran changed the title SQL Injection USH-002 - SQL Injection Mar 6, 2017
@rjmackay
Copy link
Contributor

rjmackay commented Jun 7, 2017

Looking at the referenced pages. We are using prepared statements, and escaping, however we're still outputting debug info in production.

We just need to fix this in the deployment scripts

@rjmackay rjmackay added this to the Replacing Kohana milestone Jun 7, 2017
@rjmackay rjmackay changed the title USH-002 - SQL Injection Disable SQL error output on production Jun 7, 2017
@rjmackay
Copy link
Contributor

rjmackay commented Jun 7, 2017

@tuxpiper could you look at this? we should be setting KOHANA_ENV=production

@rjmackay
Copy link
Contributor

rjmackay commented Jun 7, 2017

Hrm. Actually maybe I'm wrong. We might still be outputting the error regardless of KOHANA_ENV. Guess we need to sanitize those errors somehow

@tuxpiper
Copy link
Member

tuxpiper commented Jun 7, 2017

yep we have KOHANA_ENV=production everywhere I checked

@rjmackay
Copy link
Contributor

rjmackay commented May 2, 2018

I've set APP_ENV and APP_DEBUG in the lumen builds now. This should be resolved once lumen gets to production

@rowasc
Copy link
Contributor

rowasc commented Sep 13, 2018

Closing as lumen is out now

@rowasc rowasc closed this as completed Sep 13, 2018
@rowasc rowasc reopened this Sep 13, 2018
@rowasc
Copy link
Contributor

rowasc commented Sep 13, 2018

Reopening this needs to be verified in prod

@rjmackay
Copy link
Contributor

rjmackay commented Nov 1, 2018

It looks like there are still some minor issues.

@rowasc rowasc added Codebase: API Indicates issue work will be in API Tech Debt Theme: Security and removed P2 - Normal labels Jun 9, 2019
@tuxpiper
Copy link
Member

tuxpiper commented Jul 1, 2019

A url like https://quakemap.api.ushahidi.io/api/v3/roles?orderby=create`d&order=desc&limit=100&offset=100 will generate an SQL error. The backtick is escaped properly but the SQL is still being returned in the error

This seems to have been resolved . I am no longer getting any SQL .

@tuxpiper
Copy link
Member

tuxpiper commented Jul 1, 2019

Tampering with post data to include an id in a new post still causes a duplicate key error.

Not only that, but if the chosen ID is not duplicate, the operation goes ahead without a hitch. I don't think choosing the ID of the post should be allowed. Looks like maybe this needs its own issue? (thoughts @rowasc? )

@rowasc
Copy link
Contributor

rowasc commented Jul 1, 2019

oh. Yea the id should not be allowed in POST operations @tuxpiper Yea a new issue would be fine for the new use case and then we can close this one

@tuxpiper
Copy link
Member

tuxpiper commented Jul 1, 2019

Closing since it's not occurring anymore .

Opened #3600 for the other anomalous behavior reported in comments.

@tuxpiper tuxpiper closed this as completed Jul 1, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Codebase: API Indicates issue work will be in API Tech Debt Theme: Security
Projects
None yet
Development

No branches or pull requests

6 participants