Check your REST parameters!

Bear GilesJanuary 13th, 2014Last Updated: January 12th, 2014

1 98 5 minutes read

I was doing research related to my ongoing “project student” series and realized that I had made one of the most common – and most easily remedied – mistakes. I wasn’t using everything I know about the webapp to push my security perimeter outwards.

I am thinking specifically about the UUID parameters. I know that every valid externally visible ID will be a UUID. I know the form of the UUIDs. So why don’t I verify that my “uuid” parameters are potentially valid UUIDs before going any further?

It’s true that the database layer won’t recognize a bad “uuid” value – but that may not be the intent of the attacker. Perhaps it’s part of a SQL injection attack. Perhaps it’s part of an XSS attack. Perhaps it’s part of an attack on my logs (e.g., by including a really long value that might cause a buffer overflow). Perhaps it’s part of something I’ve never heard of. It doesn’t matter – I will always be stronger by eliminating known-invalid data as quickly as possible.

Utility Method

The utility method to determine whether a value is a possible UUID uses a simple regex pattern.

public final class StudentUtil {
    private static final Pattern UUID_PATTERN = Pattern
            .compile("^\\p{XDigit}{8}+-\\p{XDigit}{4}+-\\p{XDigit}{4}-\\p{XDigit}{4}+-\\p{XDigit}{12}$");

    /**
     * Private constructor to prevent instantiation.
     */
    private StudentUtil() {

    }

    public static boolean isPossibleUuid(String value) {
        return value != null && UUID_PATTERN.matcher(value).matches();
    }
}

If we want to be aggressive we could carefully select our UUIDs so they have additional properties that we can check. For instance the corresponding BigInteger could always have a remainder of 3 mod 17. It’s unlikely an attack would know this and we would have warning when somebody is probing our system. An even more sophisticated approach would use a different property for each class of UUID, e.g., a ‘course’ UUID might be 3 mod 17 while a ‘student’ UUID is 5 mod 17.

Unit Test

It’s easy to go overboard on our tests but a minimal set would be checking for non-hex digits,
too many or too few values, an empty string, and a null value.

public class StudentUtilTest {

    @Test
    public void testValidUuid() {
        assertTrue(StudentUtil.isPossibleUuid("63c7d688-705c-4374-937c-6628952b41e1"));
    }

    @Test
    public void testInvalidUuid() {
        assertTrue(!StudentUtil.isPossibleUuid("63c7d68x-705c-4374-937c-6628952b41e1"));
        assertTrue(!StudentUtil.isPossibleUuid("63c7d68-8705c-4374-937c-6628952b41e1"));
        assertTrue(!StudentUtil.isPossibleUuid("63c7d688-705c4-374-937c-6628952b41e1"));
        assertTrue(!StudentUtil.isPossibleUuid("63c7d688-705c-43749-37c-6628952b41e1"));
        assertTrue(!StudentUtil.isPossibleUuid("63c7d688-705c-4374-937c6-628952b41e1"));
        assertTrue(!StudentUtil.isPossibleUuid("63c7d688-705c-4374-937c-6628952b41e1a"));
        assertTrue(!StudentUtil.isPossibleUuid("63c7d688-705c-4374-937c-6628952b41e"));
        assertTrue(!StudentUtil.isPossibleUuid(""));
        assertTrue(!StudentUtil.isPossibleUuid(null));
    }
}

REST Server

The REST server should check the UUID value for all methods that require one. It’s safe to log the request parameter after we’ve verified it’s a well-formed UUID but still need to be careful about logging unsanitized values in the request.

@Path("/{courseId}")
    @GET
    @Produces({ MediaType.APPLICATION_JSON, MediaType.TEXT_XML })
    public Response getCourse(@PathParam("courseId") String id) {

        Response response = null;
        if (!StudentUtil.isPossibleUuid(id)) {
            response = Response.status(Status.BAD_REQUEST).build();
            LOG.info("attempt to use malformed UUID");
        } else {
            LOG.debug("CourseResource: getCourse(" + id + ")");
            try {
                Course course = finder.findCourseByUuid(id);
                response = Response.ok(scrubCourse(course)).build();
            } catch (ObjectNotFoundException e) {
                response = Response.status(Status.NOT_FOUND).build();
                LOG.debug("course not found: " + id);
            } catch (Exception e) {
                if (!(e instanceof UnitTestException)) {
                    LOG.info("unhandled exception", e);
                }
                response = Response.status(Status.INTERNAL_SERVER_ERROR).build();
            }
        }

        return response;
    }

An obvious improvement is to move this check (and the exception catchall) into an AOP wrapper to all service methods. This will simplify the code and go a long way towards guaranteeing that the checks are always performed. (I’m not using it in Project Student at the moment since the webservice server layer doesn’t currently have Spring dependencies.)

You can make a strong opsec argument that the REST methods should return a NOT_FOUND response instead of a BAD_REQUEST response in order to reduce information leakage.

Webapps

The details differ but we should do the same thing with webapps even if they’re just shallow front-ends to REST services. Whenever there’s a UUID, no matter it’s source, it should be checked before it is used.

Filters

There is a school of thought that security should be handled separately from the application – that the best security is knit in at deployment (via filters and AOP) instead of being baked into the application. Nobody is suggesting that app developers should ignore security considerations, just that checks like I discussed above are clutter that distract the developer and aren’t reliable since it’s easy for a developer to overlook. They would recommend using AOP or a filter instead.

It’s straightforward to write a filter that does the same work as the code above:

public class RestParameterFilter implements Filter {
    private static final Logger LOG = Logger.getLogger(RestParameterFilter.class);
    private static final Set<String> validNouns = new HashSet<>();

    /**
     * @see javax.servlet.Filter#init(javax.servlet.FilterConfig)
     */
    @Override
    public void init(FilterConfig cfg) throws ServletException {

        // learn valid nouns
        final String nouns = cfg.getInitParameter("valid-nouns");
        if (nouns != null) {
            for (String noun : nouns.split(",")) {
                validNouns.add(noun.trim());
            }
        }
    }

    /**
     * @see javax.servlet.Filter#doFilter(javax.servlet.ServletRequest,
     *      javax.servlet.ServletResponse, javax.servlet.FilterChain)
     */
    @Override
    public void doFilter(ServletRequest req, ServletResponse resp, FilterChain chain) throws IOException,
            ServletException {

        HttpServletRequest hreq = (HttpServletRequest) req;
        HttpServletResponse hresp = (HttpServletResponse) resp;

        // verify the noun + uuid
        if (!checkPathInfo(hreq, hresp)) {
            return;
        }

        // do additional tests, e.g., inspect payload

        chain.doFilter(req, resp);
    }

    /**
     * @see javax.servlet.Filter#destroy()
     */
    @Override
    public void destroy() {
    }

    /**
     * Check the pathInfo. We know that all paths should have the form
     * /{noun}/{uuid}/...
     * 
     * @param req
     * @return
     */
    public boolean checkPathInfo(HttpServletRequest req, HttpServletResponse resp) {
        // this pattern only handles noun and UUID, no additional parameters.
        Pattern pattern = Pattern.compile("^/([\\p{Alpha}]+)(/?([\\p{XDigit}-]+)?)?");
        Matcher matcher = pattern.matcher(req.getPathInfo());
        matcher.find();

        // verify this is a valid noun.
        if ((matcher.groupCount() >= 1) && !validNouns.contains(matcher.group(1))) {
            // LOG.info("unrecognized noun");
            LOG.info("unrecognized noun: '" + matcher.group(1) + "'");
            resp.setStatus(HttpServletResponse.SC_BAD_REQUEST);
            return false;
        }

        // verify this is a valid verb.
        if ((matcher.groupCount() >= 4) && !StudentUtil.isPossibleUuid(matcher.group(4))) {
            LOG.info("invalid UUID");
            resp.setStatus(HttpServletResponse.SC_BAD_REQUEST);
            return false;
        }

        return true;
    }
}

There’s no reason why we can’t also inspect the payload. For instance we can verify that dates, phone numbers and credit card numbers are properly formed; or that names only include letters (including non-Latin characters like ñ), spaces and apostrophes. (Think “Anne-Marie Peña O’Brien”.) It’s important to remember that these checks are not for ‘valid’ data – it’s to eliminate clearly ‘invalid’ data.

We must add the filter to our web.xml file.

web.xml

<filter>
    <filter-name>REST parameter filter</filter-name>
    <filter-class>com.invariantproperties.sandbox.student.webservice.security.RestParameterFilter</filter-class>
     <init-param>
        <param-name>valid-nouns</param-name>
        <param-value>classroom,course,instructor,section,student,term,testRun</param-value>
    </init-param>
</filter>

<filter-mapping>
    <filter-name>REST parameter filter</filter-name>
    <servlet-name>REST dispatcher</servlet-name>
</filter-mapping>

ModSecurity

It’s easy to write a filter for simple elements like phone numbers and names but plaintext fields are another matter. These fields need the maximum flexibility while at the same time we want to minimize the risk of XSS and other attacks.

A good resource along these lines is ModSecurity. This was originally an Apache module but it has been adopted by Trustwave Spider Labs. It sits on the web server – not the webapp – and inspects the data crossing it. A recent port (in summer 2013) allows it to be set up using a servlet filter instead of an external reverse proxy. (It uses JNI to instrument the containing appserver.)