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

DOL_DOCUMENT_ROOT now needed for analyseVarsForSqlAndScriptsInjection #29707

Conversation

fappels
Copy link
Contributor

@fappels fappels commented May 17, 2024

No description provided.

}

// Include the conf.php and functions.lib.php and security.lib.php. This defined the constants like DOL_DOCUMENT_ROOT, DOL_DATA_ROOT, DOL_URL_ROOT...
require_once 'filefunc.inc.php';
Copy link
Member

@eldy eldy May 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Security test on input variables should be executed before any manipulating or using them.
So filefunc.inc.php should be after the analyseVarsForSqlAndScriptsInjection
What needs to move this before ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I came in some situation with fatal error, think was website module. Can't reproduce. When I was in situation and debug I saw DOL_DOCUMENT_ROOT not defined here:

include_once DOL_DOCUMENT_ROOT.'/core/lib/functions2.lib.php';

Issue introduced here: 1b2bad3

@eldy eldy added the Discussion Some questions or discussions are opened and wait answers of author or other people to be processed label May 21, 2024
@eldy eldy closed this in 3fcb281 May 22, 2024
@fappels fappels deleted the develop_fix_undefined_DOL_DOCUMENT_ROOT branch May 22, 2024 07:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Discussion Some questions or discussions are opened and wait answers of author or other people to be processed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants