Security Exploit Fix

bradymiller wrote on Saturday, January 28, 2012:

Hi,

Was hoping to get some feedback on the following security exploit fix to sanitize directory and file names (the exploit is due to get published mid next week, so plan to get out a patch with a fix before then):
http://github.com/bradymiller/openemr/commit/89eff47119b5f685c1d5a707d72b3290dd385713

My questions are:
1) Should we make a function for this, since will likely be needing it in more places. If so, was gonna place in globals.php directly (this code is used to sanitize the sites variable)
2) Is this overly aggressive; are their other characters that should be allowed in directory and file names?

thanks,
-brady

bradymiller wrote on Saturday, January 28, 2012:

Hi,

Centralized the filename/directory illegal character checking to a function. Here’s the new commit: http://github.com/bradymiller/openemr/commit/87e1c13be89a39e4921c53e6db7a241ae7403f18

Plan to commit this Sunday if no objections, so have time to get the next patch out before this security exploit is made public.

-brady

yehster wrote on Saturday, January 28, 2012:

Brady,
Restricting the acceptable formname values might have an impact on ZH’s specialty wise form installation feature as I’m not sure how they track the directory structure.  I think it’s ok as they don’t seem to be using the request variable to determine directory location, but they would know better.

bradymiller wrote on Saturday, January 28, 2012:

Hi,

Rebased and placed Kevin’s commit here:
http://github.com/bradymiller/openemr/commits/security-exploit-fix_3

plan to commit this unless somebody brings up any issues

-brady

bradymiller wrote on Sunday, January 29, 2012:

Hi,

Above is testing very well, so committed this to sourceforge. Expediting this because it needs to be released in a patch within the next several days.

-brady