The problem: if a programmer forgets to DoubleApos a variable used as part of a SQL statement (it happens all the time), it is often possible for a user of the site to pass in fragments of SQL, thereby constructing arbitrary queries that let them get any information they want from the database.
The immediate solution: install Branimir and Carsten's SQL Smuggling Filter.
The permanent solution: as soon as the new database API is ready in ACS 3.4 (within the next two weeks) change all queries to use it. The new API uses bind variables, making it impossible for users to construct SQL. It also relieves you of the need to get and release database handles.
Once all queries have been converted to the new API (you'll know everything is converted as soon as there are no more occurrences of "set db " in your code), you can remove Branimir and Carsten's filter (thus saving your server from the hardship of doing a regexp on every single form input passed in).
The problem: unless we prevent the user from doing so, the user can set a variable called tmpfile when they're doing a file upload. The value of tmpfile is supposed to be set by our scripts to be the location where AOLserver temporary places the file upon upload. Then, typically, tmpfile is copied to a permanent file or to the database. We don't want the user manually setting tmpfile to /etc/passwd.
Solution: put the procedure check_for_form_variable_naughtiness (Patch 12) into your private tcl directory or into /packages/acs-core/utilities-procs.tcl. Then, modify all procedures that set variables (Patch 13) to use this procedure.
The problem: users may have access to more files in your filesystem than you intended. If they specify a directory name that contains one or more occurrences of "../" -- and if you forget to check for it -- scripts that display (or even "rm") files on your server may allow access to or modification of any file that nsadmin has permissions to read, write, and/or execute.
The immediate solution: install DVR's User Input Filter (this filter will only work after you've installed the procedures in Step 2). The filters included in the script check the directory names for all known ACS variables that are directories. However, if your custom module includes user specification of directories, you'll have to add a line like "ad_set_typed_form_variable_filter /your_module/* {your_variable dirname}".
The long-term solution: Chrooting your web server will limit the files that can be accessed. It's also probably best to keep DVR's script in place long-term because it does a number of useful things in addition to directory name checking. However, now that the glorious Oracle8i makes it so easy to handle BLOB/CLOB storage, there's not much reason to put files in the file system instead of into Oracle. We should modify the modules of the ACS (e.g. ecommerce) that store things in the file system and put everything feasible into the database.
The problem: ADP pages allow the execution of any command that can be executed from within a Tcl script. (This includes commands like [exec rm ...].) Many sites allow users to edit ADP pages. Even if these users are admin users, you don't want them executing arbitrary SQL statements or shell commands (on some sites, not even administrators are supposed to see the credit card numbers). Also, you never know how careful administrators are with keeping their passwords private.
The solution: Disallow command execution in ADP pages. If you want an ADP page to contain the output of a procedure, set a variable to have that value (which will work as long as the ADP page is ns_adp_parse'd from a Tcl script). Then the editor of the page can refer to the value of the variable with <%= $variable_name %>. The removal of command execution ability is accomplished via Patch 1. Note: your current ADP pages will also have to be edited to remove function calls. (If you want to be a little less string, modify the patch to include a list of acceptable functions.)
The problem: if you set user_id [ad_verify_and_get_user_id] and then, one line later, set_the_usual_form_variables, it is possible that user_id will be overwritten by a form variable called user_id. This can give the user access to see information and perform actions allowed to a different registered user (e.g., a site administrator).
The immediate solution: unfortuately a simple filter on the whole site to prevent overwriting of user_id is inadequate because sometimes the user_id from the cookie is set to be admin_id (
The long-term solution: We should be using ad_page_variables everywhere. Not only is it more convenient (you can set default values for unsupplied variables in one step) and self-documenting (each variable is listed as an argument), it's much safer: no variables can be set unless you allow them to be set.
The problem: the naughtiness filter is not applied if form input comes through ns_queryget (an AOLserver function) instead of through our own variable-setting procedures. Note: Branimir and Carsten's SQL Smuggling Filter will catch dangerous SQL fragments, so we only have to worry about values that are "naughty" in some other way.
The immediate solution: apply Patch 2 to get rid of the one dangerous ns_queryget in the ACS and grep for occurrences of ns_queryget in your custom code.
The long-term solution: Avoid or rewrite ns_queryget.
These are patches that are not be covered by the above sections, so go ahead and apply them:
set_the_usual_form_variables before setting any other variables, or you can add a DVR filter.