History | Log In     View a printable version of the current page.  
Issue Details (XML | Word | Printable)

Key: UP-647
Type: Bug Bug
Status: Open Open
Priority: Major Major
Assignee: Unassigned
Reporter: Robin West
Votes: 0
Watchers: 0
Operations

If you were logged in you would be able to see more operations.
uPortal

RDBMUserLayoutStore SQL escaping

Created: 21/Sep/04 11:45 AM   Updated: 26/Oct/07 11:38 AM
Component/s: Preferences
Affects Version/s: 2.3.3, 2.3.4, 2.3.5, 2.4, 2.4.1, 2.4.2, 2.5.0 M1, 2.5.1 RC1, 2.5.0 RC1, 2.5.0 RC3, 2.5.0 RC2, 2.5.0 GA, 2.5.1 RC2, 2.5.2 RC1, 2.5.1 RC3, 2.5.1 GA, 2.5.3 RC1, 2.5.2 GA, 2.5.3 RC2, 2.5.3 RC3, 2.5.3 GA
Fix Version/s: 2.5.4

Original Estimate: Unknown Remaining Estimate: Unknown Time Spent: Unknown

Bugzilla Id: 1678


 Description  « Hide
This may be a database-specific problem but I have verified it with both Postgres 7.3.4 and MySQL. Confirmed in uPortal 2.3.3 and 2.3.4, probably in earlier releases as well.

If the user's name contains a single-quote (eg "O'Brien") the user's layout fails to save. It would appear that an error occurs when the "userName" attribute is saved in the UP_SS_USER_PARM table.

Steps to reproduce:
1) create a user using ant md5passwd.
2) update the user's record in UP_PERSON_DIR table to include a first and last
name. Put an single quote in the last name, eg. "O'Brien".
3) Log in to uPortal with the user.
4) Go to preferences and modify the layout (add a tab).
5) Save preferences.
6) Logout.
7) Log in again with the same user. The layout changes were not saved.

There is no error reported in the portal logs - it would appear that the database error is being caught and thrown away. However, database transaction logs will show something similar to the following:

postgres.log:Sep 19 11:55:56 kil-udb-1 postgres[22352]: [4172] ERROR: parser:
parse error at or near "Brien" at character 125

I corrected this problem by stripping single quotes out of the userName when it gets stored in UserInstance.processUserLayoutParameters():

// DAL remove single quotes in name field RW 21-Sep-2004
// original code: themePrefs.putParameterValue("userName", userName);
            themePrefs.putParameterValue("userName", userName.replaceAll
("'", ""));
// DAL end

There is probably a better way that escapes the single-quotes properly, but I didn't have a lot of time to fiddle with it.

-Robin

 All   Comments   Work Log   Change History   FishEye      Sort Order:
Brad Johnson [22/Sep/04 10:18 AM]
Looks like around Line 2483 in RDBMUserLayoutStoresetTheme.
StylesheetUserPreferences in uPortal 2.3.4 there is a query built by
contatenating strings together:

// insert
sQuery = "INSERT INTO UP_SS_USER_PARM
(USER_ID,PROFILE_ID,SS_ID,SS_TYPE,PARAM_NAME,PARAM_VAL) VALUES (" + userId
+ "," + profileId + "," + stylesheetId + ",2,'" + pName + "','" +
tsup.getParameterValue(pName) + "')";

The value returned by tsup.getParameterValue(pName) should be sql escaped using
RDBMServices.sqlEscape(). There may be other problems like this in the same file.

Is there still a need to support JDBC drivers that can't do prepared statements?
Using prepared statements would eliminate problems like this...


Robin West [23/Sep/04 08:02 AM]
I did some more work on this and actually it needs to be fixed in eight
places...

RDBMUserLayoutStore.setStructureStylesheetUserPreferences (2 places)
RDBMUserLayoutStore.setThemeStylesheetUserPreferences (2 places)
AggregatedUserLayoutStore.setStructureStylesheetUserPreferences (2 places)
AggregatedUserLayoutStore.setThemeStylesheetUserPreferences (2 places)

-Robin


Robin West [23/Sep/04 12:31 PM]
Ugh ... one more place to be fixed:

AggregatedUserLayoutStore.setAggregatedLayout() - where it calls
person.getFullName() in the insert into UP_USER_LAYOUT_AGGR.

-Robin


Ken Weiner [23/Sep/04 03:22 PM]
The best way to fix this is to change all the use of Statements within
RDBMUserLayoutStore to PreparedStatements so special characters are escaped
properly. Unfortunately, RDBMUserLayoutStore is a pretty complicated class so
the effort to do this won't be trivial. There isn't enough time to do it for
the 2.3.5 and 2.4 release. Maybe by the 2.4.1 release...

Any volunteers?


Andrew Petro [16/Apr/05 08:22 PM]
Pushing back this issue so that this change does not conflict with changes for DLM / from the RDBMServices changes that we're QAing.

Andrew Petro [04/May/05 03:26 PM]
Serious issue. Needs to be addressed for 2.5.0 RC2.

Robin West [26/Oct/07 11:38 AM]
I was just reapplying some local mods and fixes to a 2.6.0-GA installation and thought I should note that RDBMUserLayoutStore is still full of unsafe SQL statements (i.e. not using PreparedStatement) as of this release.