Skip to content

Commit

Permalink
Security: Rename htaccess file by replacing case-insensitively
Browse files Browse the repository at this point in the history
  • Loading branch information
@AngelFQC
AngelFQC committed Jul 13, 2023
1 parent 3650e44 commit dc7bfce
Showing 1 changed file with 4 additions and 2 deletions.
6 changes: 4 additions & 2 deletions main/inc/lib/fileUpload.lib.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,15 +30,17 @@ function php2phps($file_name)
}

/**
* Renames .htaccess & .HTACCESS to htaccess.txt.
* Renames .htaccess & .HTACCESS & .htAccess to htaccess.txt.
*
* @param string $filename
*
* @return string
*/
function htaccess2txt($filename)
{
return str_replace(['.htaccess', '.HTACCESS'], ['htaccess.txt', 'htaccess.txt'], $filename);
$filename = strtolower($filename);

This comment has been minimized.

Sign in to view

Copy link
@NicoDucou

NicoDucou Sep 18, 2023

Member

@AngelFQC
This "strtolower" generates many problem because the function htaccess2txt is called by "disable_dangerous_file" which is passed to all files, so all the files and directory on the plateform became lowercase. Which causes problem when importing a scorm for example that contains sub folder with capital letters and hard link in the HTML files of the scorm (Refs BT#21023.
It also generates problem when reimporting a course that has Capital letters in the folders so the path do not work anymore when reimporting. See issue #4872

This comment has been minimized.

Sign in to view

Copy link
@NicoDucou

NicoDucou Sep 18, 2023

Member

I commented the line that passes the filename to lowercase and all works now but I'm not sure if the security fix that you added is still working so I did not send it to Chamilo 1.11.x and wait for your comment about this.

This comment has been minimized.

Sign in to view

Copy link
@LudiscapeXApi

LudiscapeXApi Sep 18, 2023 •
edited
Loading

Contributor

J'ai corrigé le code ici : (et testé l'import SCORM aussi)
1.11.x...LudiscapeXApi:chamilo-lms:patch-17

This comment has been minimized.

Sign in to view

Copy link
@NicoDucou

NicoDucou Sep 18, 2023

Member

@AngelFQC can we merge the patch indicated by @LudiscapeXApi ?

This comment has been minimized.

Sign in to view

Copy link
@NicoDucou

NicoDucou Sep 18, 2023

Member

Finally it was fixed with this commit f680d7d

return str_replace('.htaccess', 'htaccess.txt', $filename);
}

/**
Expand Down

0 comments on commit dc7bfce

Please sign in to comment.