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

Security Fix #37

Open
wants to merge 15 commits into
base: master
Choose a base branch
from
Open

Security Fix #37

wants to merge 15 commits into from

Conversation

dushankw
Copy link

@dushankw dushankw commented Apr 1, 2015

Hi,

I've used PIP for a personal project and made quite a lot of changes/fixes (along with my work mates) and I intend to contribute those upstream. We added functionality, upgraded the database stuff to use PDO and did a few other things.

The first change I'm submitting is a security fix, which protects the framework from https://www.owasp.org/index.php/PHP_File_Inclusion

EDIT: See also https://www.owasp.org/index.php/Testing_for_Local_File_Inclusion

Please consider merging...

Thanks,
Dushan

@OzairP
Copy link

OzairP commented Jun 15, 2015

This doesn't look like it supports sessions. Can you clarify?

@dushankw
Copy link
Author

I'm not sure I follow, fix #37 specifically addresses the problem where you could specify a fully qualified path to a file on the servers disk and the router would include it, for example site.com/../../../etc/passwd

https://www.owasp.org/index.php/Testing_for_Local_File_Inclusion
https://en.wikipedia.org/wiki/File_inclusion_vulnerability
https://www.owasp.org/index.php/PHP_File_Inclusion

The session handling is done in index.php in the same way it always was, I've not changed that (however, I intend to put some more code in soon to prevent session fixation attacks and to generate stronger id's)

@dushankw
Copy link
Author

  • The rewrite forces everything through index.php (which starts the session)
  • index.php includes the base classes for the model/controller etc.
  • Then each user generated model/controller extends the base one (and thus accesses all its members)
  • For this reason you don't need a session_start() in each file

Was that what you meant?

@mehdi-mashayekhi
Copy link

a problem :

when my script is in subfolder dont load style.css loool

plaease check BASE_URL

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.

3 participants