Date
03/21/2013
How to Join
Click here for information about joining the meeting in person or remotely.
By Browser
By telephone
- US telephone number: +1 201.479.2627
Agenda
- Quickly review previous meeting minutes (5 min)
- OpenMRS Security – reviewing feedback from January 2013 FLOSSHacks event
- After-action review & next week's agenda (5 min)
In Attendance
Minutes
Developers Forum 2013-03-21
Agenda
- OpenMRS Security – reviewing feedback from FLOSSHacks event
Attendees
Minutes
Vulnerabilities
===============
XSS
* Patient Display
* Patient Name
* Dimension Name
* Concept Name
* Form Fields name/description
* Locations
Sessions
* JSESSIONIDs exposed in URL
* JSESSIONID assigned before login, used as session cookie after login.
* Lack of HTTPOnly flag on JSESSIONID cookies
Runtime Properties
* Created as world readable
* Unsafe location (/usr/share/tomcat7/.OpenMRS/ is bad, /var/lib/tomcat7/webapps/openrmrs/ would be better)
Passwords
* Forgotten Password Form leaks valid usernames
* Weak login brute force mitigation + bug in parsing security.loginAttemptsAllowedPerIP
SQL Injection
* ConceptValidatorChangeSet.isNameUniqueInLocale
XXE
* UpdateFileParser.java
________________________________
Vulnerabilities
===============
=== Issue #1: Reflected XSS in Patient Display ===
Credit: Kevin Jacobs
=== Issue #2: Multiple Stored XSS via Patient Name === (Steve)
Credit: Kevin Jacobs
The create patient flow is allows stored XSS when using the following
name (script is executed when loading mdrtbEditPatient.form, and
potentially other pages that display the patient name).
"><script>alert("xss")</script>
Other places where patient/user name injects script:
module/reporting/reports/reportHistory.form (xss in automatically populated into the "Requested By" dropdown)
admin/users/user.form (Which Person? textbox autocomplete fires it)
admin/encounters/encounter.form (same dropdown as above. Just fix this control)
patientDashboard.form (multiple places where the name is displayed. Main page, demographics tab.
Every page when logged in as the xss username ("Currently logged in as <script>..") (meh..)
Privilege escalation scenario:
Setup: Admin creates "Person" record and links it to a User account.
1. User can change his/her name to include an XSS string (steal session cookie)
2. User gets admin to visit their "Person" profile, i.e.
admin/person/person.form?personId=93483. Script could steal admin
cookie and send it to a malicious url.
=== Issue #3: Multiple Stored XSS via Dimension Name ===
Credit: Kevin Jacobs
Pages:
module/reporting/indicators/editCohortDefinitionDimension.form
module/reporting/parameters/queryParameter.form
module/reporting/indicators/manageDimensions.form (executes previously injected Dimension Name scripts)
reporting/indicators/editCohortDefinitionDimension: name and description parameters. XSS
=== Issue #4: Multiple Stored XSS via Concept Name ===
Credit: Kevin Jacobs
patientDashboard.form (Graphs tab can display XSS'd Concept Name (search Find Concepts))
dictionary/concept.form (viewing XSS'd Concept Name)
=== Issue #5: JSESSIONIDs exposed in URL ===
Credit: Timothy D. Morgan
Upon first hitting the app:
=== Issue #6: Session Fixation ===
Credit: Timothy D. Morgan
JSESSIONID assigned before login, used as session cookie after login.
=== Issue #7: Locally Exposed Database Credentials ===
Credit: Timothy D. Morgan
This file contains application database credentials and is created world readable:
/usr/share/tomcat7/.OpenMRS/openmrs-runtime.properties
=== Issue #8: Stored XSS via Custom Form Fields ===
credit: Lauren
Partial POST:
POST /openmrs/admin/forms/formEdit.form HTTP/1.1
Host: 192.168.56.101
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:18.0)
Gecko/20100101 Firefox/18.0
Accept: text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8
Accept-Language: en-US,en;q=0.5
Accept-Encoding: gzip, deflate
Cookie: JSESSIONID=5ADE5893C3F2E6B1FF89040BA35740AD;
Connection: keep-alive
Content-Type: multipart/form-data;
boundary=---------------------------4850512294080817911715376466
Content-Length: 1019
-----------------------------4850512294080817911715376466
Content-Disposition: form-data; name="name"
<script>alert(document.cookie)</script>
-----------------------------4850512294080817911715376466
...
Fields name and description verified to allow stored injection.
=== Issue #9: Forgotten Password Form Leaks Valid Usernames ===
credit: Timothy D. Morgan
Submitting invalid username returns "Invalid user or the secret
question has not been set. Please contact an administrator for help
resetting your password." while a valid username (but no secret
question) yields "Invalid user or the secret question has not been
set. Please contact an administrator for help resetting your
password.".
(Issue #10 done: TRUNK-3930)
=== Issue #10: Weak Login Brute Force Mitigation ===
credit: Kevin Jacobs
Login lock outs (LoginServelet.java, lines 82+):
Lock out after 100 attempts. Timer is reset after 5 minutes.
Recommend lockout after 5, (email to admin?).. require admin intervention to unlock.
Note: Default is defined in WebConstants.java can override the static 100 default:
public static String GP_ALLOWED_LOGIN_ATTEMPTS_PER_IP = "security.loginAttemptsAllowedPerIP";
But the code expects an int, parsing fails, and sets to 100 attempts by default.
=== Issue #11: Stored XSS in Location Pages ===
credit: Kevin Jacobs
admin/locations/location.form - Create new location (<script>alert(1)</script> as name). Load this form, script is ran from Parent Location dropdown box.
admin/locations/locationTag.list - name, desc parameters
admin/locations/hierarchy.list - previously stored location name parameter.
Other Observations
==================
Issue #12
=== Unsafe Location for Runtime Properties Under Linux ===
credit: Timothy D. Morgan
Under Debian Linux using the .war file, the application expects to be
able to write runtime properties and other information to:
/usr/share/tomcat7/.OpenMRS/openmrs-runtime.properties
This directory is not writable by the tomcat7 user, and for good
reason. This could be viewed as a simple bug, fixable by a
permissions change, but in fact this leads to a security risk. The
tomcat user should not have access to write to things under /usr. If
it did, a single malicious/compromised web application may be able to
escalate privileges. Certainly the .OpenMRS directory is somewhat
isolated, and a smart user would provide tomcat with permissions only
to this directory. However, it isn't so far fetched to imagine a user
doing:
chown -R tomcat7 /usr/share/tomcat7/
Ultimately, the runtime properties should be written to a sane place,
such as /var/lib/tomcat7/webapps/openmrs/. Under normal conditions,
tomcat7 would have access to this. I have attempted to override the
default behavior based on the environment variable mentioned here:
But this did not work. It seems to ignore the environment variable.
Consider using Java System properties to determine the path to the
webapps dir, or the deployed application's directory and use that instead.
Issue #13
=== Potential Second Order SQLi in ConceptValidatorChangeSet.isNameUniqueInLocale ===
credit: Timothy D. Morgan
The escapeSqlWildcards method likely only escapes wildcard characters, not ' and the like.
private boolean isNameUniqueInLocale(JdbcConnection connection, ConceptName conceptName, int conceptId) {
int duplicates = getInt(connection,
"SELECT count(*) FROM concept_name cn, concept c WHERE cn.concept_id = c.concept_id AND (cn.concept_name_type = '"
+ ConceptNameType.FULLY_SPECIFIED
+ "' OR cn.locale_preferred = '1') AND cn.voided = '0' AND cn.name = '" + HibernateUtil.escapeSqlWildcards(conceptName.getName(), connection.getUnderlyingConnection())
+ "' AND cn.locale = '"
+ HibernateUtil.escapeSqlWildcards(conceptName.getLocale().toString(), connection
.getUnderlyingConnection()) + "' AND c.retired = '0' AND c.concept_id != " + conceptId);
return duplicates == 0;
}
Issue #14
=== Potential XXE via update.rdf Download ===
credit: Timothy D. Morgan
Update RDF files seem to be downloaded via HTTPS, which authenticates
them. However, if these are ever distributed from an insecure source
or over an insecure channel, then they would likely be parsed with XML
external entity support enabled. Relevant code in
UpdateFileParser.java:
/**
* Parse the contents of the update.rdf file.
*
* @throws ModuleException
*/
public void parse() throws ModuleException {
StringReader stringReader = null;
try {
Document updateDoc = null;
try {
stringReader = new StringReader(content);
InputSource inputSource = new InputSource(stringReader);
inputSource.setSystemId("./");
DocumentBuilderFactory dbf = DocumentBuilderFactory.newInstance();
DocumentBuilder db = dbf.newDocumentBuilder();
updateDoc = db.parse(inputSource);
}
catch (Exception e) {
log.warn("Unable to parse content");
throw new ModuleException("Error parsing update.rdf file: " + content, e);
}
...
Issue #15
=== Lack of HTTPOnly Flag ===
Set the HTTPOnly flag on JSESSIONID cookies. This will marginally
help to mitigate XSS exploits.
TODOs
Transcripts
- Backchannel IRC transcript
- Audio recording of the call: Listen online or download - available after the meeting
Jamie Thomas
Burke Mamlin