CVE-2016-10007 and CVE-2016-10008 - 2 SQL injection vulnerabilities in dotCMS, blacklist defence bypass

Background

I had already reported some SQL injection vulnerabilities to dotCMS [1].

Previous reports: Multiple SQL injection vulnerabilities in dotCMS (8x CVE Full Disclosure).

How previous SQL injections are fixed?

I was curious as to how they fixed it. All research is done dotCMS version 3.7.

Previously, the sort orderby parameter went directly from a request to SQL query. Now there are 2 new "sanitize" functions in place:

parameter1 = SQLUtil.sanitizeSortBy(parameter1)
parameter2 = SQLUtil.sanitizeParameter(parameter2)

How does it really work, then? File: ./dotCMS/src/main/java/com/dotmarketing/common/util/SQLUtil.java, version 3.7:

...
public class SQLUtil {

    private static final Set<String> EVIL_SQL_WORDS = ImmutableSet.of(
        "select ", "insert ", "delete ", "update ",
        "replace ", "create ", " distinct ", " like ",
        " and ", " or ", " limit ", " group ", " order ", " as ",
        " count ", "drop ", "alter ", "truncate ", "declare ",
        " where ", "exec ", "--", " procedure ", "pg_",
        "lock ", "unlock ", "write ", " engine ",
        "null", "not ", " mode ", "set ", ";" );

    private final static Set<String> ORDERBY_WHITELIST= ImmutableSet.of(
            "title","filename", "moddate", "tagname","pageUrl",
            "category_name","category_velocity_var_name",
            "mod_date","structuretype,upper(name)","upper(name)",
            "category_key", "page_url","name","velocity_var_name",
            "description","category_","sort_order","hostName",
            "keywords","mod_date,upper(name)");

    // ... removed ...

    public static String sanitizeSortBy(String parameter){

        if(!UtilMethods.isSet(parameter)){//check if is not null
            return "";
        }

        String testParam=parameter.replaceAll(" asc", "").replaceAll(" desc", "").replaceAll("-", "").toLowerCase();
        if(ORDERBY_WHITELIST.contains(testParam)){
            return parameter;
        }

        // ... removed ... (Exception and Logging)
        return "";
    }

    public static String sanitizeParameter(String parameter){

        if(!UtilMethods.isSet(parameter)){//check if is not null
            return "";
        }
        parameter = StringEscapeUtils.escapeSql(parameter);

        for(String str : EVIL_SQL_WORDS){
            if(parameter.toLowerCase().contains(str)){//check if the order by requested have any other command
                // ... removed ... (Exception and Logging)
                return "";
            }
        }

        return parameter;
    }
}

So, sanitizeSortBy is meant to work as a whitelist and sanitizeParameter as a blacklist. Both of them have problems.

Problems in sanitizeSortBy function

There are at least 2 problems, we can create SQL errors with both of them and it leads to "systematic information disclosure" - which is not too huge a problem for open source project, but gives an idea about mindset and quality.

First, written the whitelist covers the whole application, it's not page/query specific. We can send parameter "category_velocity_var_name" as a valid order by parameter for all pages, but this field does not exist in those tables and we have SQL error.

com.dotmarketing.exception.DotRuntimeException: could not resolve property: category_velocity_var_name of: com.dotmarketing.portlets.templates.model.Template [select asset from asset in class com.dotmarketing.portlets.templates.model.Template, inode in class com.dotmarketing.beans.Inode, identifier in class com.dotmarketing.beans.Identifier, versioninfo in class com.dotmarketing.portlets.templates.model.TemplateVersionInfo where asset.inode = inode.inode and asset.identifier = identifier.id and versioninfo.identifier=asset.identifier and identifier.hostId = '48190c8c-42c4-46af-8d1a-0cd5db894797' and asset.type='template' and asset.inode=versioninfo.workingInode and versioninfo.deleted = 'false' order by asset.category_velocity_var_name]

Second problem: the whitelist check is not case-sensitive but field names in database are. If we send value modDatE instead of modDate, we can see similar error:

com.dotmarketing.exception.DotRuntimeException: could not resolve property: modDatE of: com.dotmarketing.portlets.links.model.Link [
select asset from asset in class com.dotmarketing.portlets.links.model.Link, inode in class com.dotmarketing.beans.Inode, identifier in class com.dotmarketing.beans.Identifier, versioninfo in class com.dotmarketing.portlets.links.model.LinkVersionInfo where asset.inode=inode.inode and asset.identifier = identifier.id and versioninfo.identifier=identifier.id and identifier.hostId = '48190c8c-42c4-46af-8d1a-0cd5db894797' and asset.type='links' and asset.inode=versioninfo.workingInode and versioninfo.deleted = 'false' order by asset.modDatE desc]

Problems in sanitizeParameter functions

First, parameters, which contains space:

"select ", "insert ", "delete ", "update ",
"replace ", "create ", " distinct ", " like ",
" and ", " or ", " limit ", " group ", " order ",
" as ", " count ", "drop ", "alter ",
"truncate ", "declare ", " where ", "exec ",
" procedure ", "lock ", "unlock ", "write ",
" engine ", "not ", " mode ", "set "

Options that I have in mind when I see this kind of list:

  • What if an attacker uses another whitespace character instead of space? For example tab (\t) or new line (\n).

    SELECT
field
FROM
TABLE

  • Instead of spaces I can use multiline comments: /**/

    select/*no*/field/*spaces*/FROM/*here*/TABLE

  • I can avoid spaces using brackets

    select(field)FROM(TABLE)

Next ones:

"pg_", "null"

It means that in this application it is illegal to search phrases which contains "pg_" or "null". This does not sound like a business-logic requirement, more like a bug.

Last ones:

"--", ";"

Well, an attacker cannot use one-line comment and multiqueries. It makes the attack code a bit longer, attacking a bit slower, and that's it. It's still possible.

In the sanitizeParameter function is also escape function call:

parameter = StringEscapeUtils.escapeSql(parameter);

The manual describes: https://commons.apache.org/proper/commons-lang/javadocs/api-2.6/org/apache/commons/lang/StringEscapeUtils.html#escapeSql(java.lang.String)

At present, this method only turns single-quotes into doubled single-quotes
("McHale's Navy" => "McHale''s Navy").
It does not handle the cases of percent (%) or underscore (_) for use in LIKE clauses.

Conclusions - sanitizeParameter is usable only if:

  • value is a string and it's located between ' signs in SQL query
  • the database engine/driver understands escaping with the same character

It does not give defence, if:

  • the value is not located between ' signs (like order by, limit, field and table names, numeric values)
  • the value is in LIKE value - it does not give SQL injection, but wildcards % and _ are not escaped.
  • the database accepts other characters as escape characters (like \)

The code is written for using PostgresSQL but actually MySQL, ORACLE and MS SQL are also supported?

Usage of sanitizeParameter function

$ ... dotcms/code/core-master-3.7/dotCMS/src/main/java/com/dotmarketing$ grep -r "sanitizeParameter" .

Is this function at least used in the correct places?

UserFactoryLiferayImpl.java

File: ./business/UserFactoryLiferayImpl.java

Used 5 times and all of them with the LIKE operator. Symbols % and _ always works as wildcards (but should not).

WebAssetFactory.java

File: ./factories/WebAssetFactory.java

Used correctly once. Another function does the escaping and forwards it to the next function where the actually dynamic SQL is built. This is really dangerous way to do that - escaping must be done where it is used.

In this WebAssetFactory.java there is another interesting thing:

public PaginatedArrayList<PermissionAsset> getAssetsAndPermissions( ...
    // ... removed ...
    query = SQLUtil.sanitizeParameter(query);
    // ... removed ...
    if(UtilMethods.isSet(query)){
        params.put("title", query.toLowerCase().replace("\'","\\\'"));
    }
    // ... removed ...
}

Seriously, what's that? "Escaping" with string-replace?

\'  =>  \\\'

For breaking it we could enter?

\\' =>  \\\\' (for database it's \\')

CategoryFactoryImpl.java

File: ./portlets/categories/business/CategoryFactoryImpl.java

The next bad-coding-example:

protected List<Category> findChildrenByFilter(String inode, String filter, String sort) throws DotDataException {
    ...
    filter = SQLUtil.sanitizeParameter(filter);
    ...
    StringBuilder sql = new StringBuilder();
    ...
    sql.append(getFilterAndSortSQL(filter, sort));
    dh.setSQLQuery(sql.toString());
    return (List<Category>) dh.list();
}

private String getFilterAndSortSQL(String filter, String sort) {
    filter = SQLUtil.sanitizeParameter(filter);
    ... filter is used in like later ...
}

So, in function findChildrenByFilter escaping for "filter" is done, then it's sent to function getFilterAndSortSQL and it does escaping again. Twice as secure? :) No. It's broken functionality in case your search phrase contains the escape character '.

InodeFactory.java

File: ./factories/InodeFactory.java

Used 1 time and in the direction parameter - which potentially leads us to new vulnerability:

public static java.util.List getInodesOfClassByConditionAndOrderBy(/* removed */) {
    // ... removed ...
    orderby = SQLUtil.sanitizeSortBy(orderby);
    direction = SQLUtil.sanitizeParameter(direction);

    try {
        // ... removed ...
        // order
        query +=  (UtilMethods.isSet(orderby) ? " order by " + orderby + "" : "");
        query += ((UtilMethods.isSet(orderby) && UtilMethods.isSet(direction)) ? " " + direction : "");
        // ... removed ...
        dh.setQuery(query);
        // ... removed ...
    }

    return new java.util.ArrayList();
}

The direction parameter should allow only 2 words: ASC or DESC. Why does it use some sanitizeParameter function there? It does not give any effect. It's an SQL injection security hole! Proof-of-Concept and details are described below.

CVE-2016-10007 - "Marketing" > Forms" page, _EXT_FORM_HANDLER_orderBy parameter

An SQL injection vulnerability in the "Marketing > Forms" screen in dotCMS before 3.7.2 (not released) and 4.1.1 allows remote authenticated attackers to execute arbitrary SQL commands via the _EXT_FORM_HANDLER_orderBy parameter.

Preconditions: the attacker must be authenticated and authorized as an administrator.

Proof-of-Concept URL (from "Admin Site" UI: "Marketing > Forms", click on some column title in the resultset table):

/c/portal/layout?p_l_id=89594b95-1354-4a63-8867-c922880107df&p_p_id=EXT_FORM_HANDLER&p_p_action=1&p_p_state=maximized&p_p_mode=view&_EXT_FORM_HANDLER_struts_action=%2Fext%2Fformhandler%2Fview_form&_EXT_FORM_HANDLER_orderBy=SQLi&_EXT_FORM_HANDLER_direction=asc

Proof-of-Concept values for parameter _EXT_FORM_HANDLER_orderBy. Precondition for this example: there must be at least 2 different rows in the resultset and ordering by name and description field should give different ordering (if they don't, use some other field names)

-- boolean true - output is ordered by name field
_EXT_FORM_HANDLER_orderBy=case when 1=1 then name else description end

-- boolean false - output is ordered by descriotion field
_EXT_FORM_HANDLER_orderBy=case when 1=0 then name else description end

Related links

CVE-2016-10008 - "Content Types > Content Types" page, _EXT_STRUCTURE_direction parameter

An SQL injection vulnerability in the "Content Types > Content Types" screen in dotCMS before 3.7.2 (not released) and 4.1.1 allows remote authenticated attackers to execute arbitrary SQL commands via the _EXT_STRUCTURE_direction parameter parameter.

Preconditions: the attacker must be authenticated and authorized as an administrator.

Proof-of-Concept URL (from "Admin Site" UI: "Content Types > Content Types", click on some column title in the resultset table):

demo.dotcms.com/c/portal/layout?p_l_id=56fedb43-dbbf-4ce2-8b77-41fb73bad015&p_p_id=EXT_STRUCTURE&p_p_action=1&p_p_state=maximized&p_p_mode=view&_EXT_STRUCTURE_struts_action=%2Fext%2Fstructure%2Fview_structure&_EXT_STRUCTURE_orderBy=velocity_var_name&_EXT_STRUCTURE_direction=SQLi

Related links

Suggestions

You.. can.. NOT.. use.. blacklists.

As I cannot see fix dotCMS 3 and latest release 3.7.1 is already one year old, then it makes sense to move on to dotCMS 4.

Vulnerability Disclosure Timeline

Timezone for dates: Tallinn/Europe

  • 2016-10-24 | me > dotCMS | SQLi Poc
  • 2016-10-25 | dotCMS > me | Thanks for PoC
    They thanked me for detailed proof-of-concepts. Additionally they asked whether would I like to do some extra pen-testing. My answer was - yes, but through Clarified Security services. Even though there was interest, this extra pentest never happened.

Silence...

  • 2016-12-19 | me > MITRE | CVE request
  • 2016-12-19 | MITRE > me | CVE-2016-10007, CVE-2016-10008
  • 2016-12-19 | me > dotCMS | Informed CVE numbers, asked status for reported issues.
  • 2016-12-19 | dotCMS > me | Low priority, not planning fixing in next release
    "Because these issue require a user to be logged into the backend we view these to be a lower risk and a lower priority. We are in the process of rewriting our entire backend UI which is why we inquired about the services your company provides. The fixes for these issue are not included in the 3.6.1 release."
  • 2016-12-19 | me > dotCMS | agreed with low priority (requires authenticated user in administrator privileges)

Silence...

  • 2017-03-03 | me > dotCMS | I can see many new releases, is it fixed? [2]
  • 2017-03-06 | dotCMS > me | No. Probably will be not addressed until the project to refactor our admin interface is completed.

Meanwhile, release for dotCMS 4

  • 2016-06-16 | dotCMS | dotCMS version 4.1.1 release [2]

More silence...

  • 2017-07-18 | me > dotCMS | As I need to publich CVEs at some point, what is the status?
  • 2017-07-21 | dotCMS > me | Fixes are available on 4.1.1. Would it be possible to wait 3 to 4 weeks so we can release 3.7.2?

And more silence...

  • 2017-10-10 | me > dotCMS | "3 to 4 weeks" passed, how it is going with 3.7.2?
  • 2017-10-17 | dotCMS > me |
    Thank you for your patience! Thank you for your email! It prompted me to push the developer to finish getting this release out the door. I will email you next week with an update.

Silence... This "next week" never arrived ;)

Since dotCMS v3.7.1 is over a year old, and since version 4.1.1 reportedly contains fixes, I decided to publish my findings.

  • 2018-02-11 | me | Full Disclosure on security.elarlang.eu
  • 2018-02-13 | me | Full Disclosure in FullDisclosure mailinglist on seclists.org [3]

Statuses for listed problems

Fixed in dotCMS version 4.1.1. (based on feedback from dotCMS, haven't verified it myself).

Theoretically this issue would be fixed in version 3.7.2 if it ever comes.

Clarification

Potential question: Why did I pay so much attention to dotCMS? Do I have something against it?

Answer: No. Initially I tested dotCMS during one a pen-test case at Clarified Security. Later (like issues described here) I just checked whether they ahd fixed the issues that were found.

References

Comments

comments powered by Disqus