web: fix various vulnerabilities #5967
Conversation
davidpanderson
commented
Dec 20, 2024
davidpanderson
commented
- Send all cookies as HttpOnly (don't let Javascript see them)
- If page sending cookie is HTTPS, make the cookie secure
- Often we have something like: $name = get_str('name'); if (!lookup($name)) { error_page("can't find $name"); } Can't do this; it can be exploited for XSS attacks. Just say 'Can't find file' or whatever
- Don't show database error messages
- Send all cookies as HttpOnly (don't let Javascript see them)
- If page sending cookie is HTTPS, make the cookie secure
- Often we have something like:
$name = get_str('name');
if (!lookup($name)) {
error_page("can't find $name");
}
Can't do this; it can be exploited for XSS attacks.
Just say 'Can't find file' or whatever
- Don't show database error messages
There was a problem hiding this comment.
Copilot wasn't able to review any files in this pull request.
Files not reviewed (12)
- html/inc/consent.inc: Language not supported
- html/inc/prefs_util.inc: Language not supported
- html/inc/user_util.inc: Language not supported
- html/inc/util.inc: Language not supported
- html/inc/util_basic.inc: Language not supported
- html/user/am_set_host_info.php: Language not supported
- html/user/buda.php: Language not supported
- html/user/manage_app.php: Language not supported
- html/user/manage_project.php: Language not supported
- html/user/sandbox.php: Language not supported
- html/user/team_forum.php: Language not supported
- html/user/team_founder_transfer_action.php: Language not supported
|
Tested. LGTM |
|
@lfield @davidpanderson The commit is broken. server_release/1.4/1.4.5...server_release/1.4/1.4.6#diff-d5fbb528304cce7cc7425fdb9fc52f69865c650a7509060720c0f797d225ade3R165 contains git diff lines in the main PHP code (<<< HEAD, ===== etc) in more than one place. Moreover, regarding is_valid_filename() - htmlspecialchars($x) is used to check for invalid characters. However, this is not appropriate for checking valid filenames. htmlspecialchars is meant for escaping HTML characters (like quotes, <, >, and &) and may incorrectly reject valid filenames with special characters. These characters are perfectly valid characters in filenames. Could we stop panicking and making new server releases one per day, and instead wait and review stuff properly? |
|
Thanks for this. Something went wrong during the cherry pick. |
