Project page: Secure Code Review
Student: Jim Berry (solotandem)
Mentor: Greg Knaddison (greggles)
Project Status
Done
- reviewed and summarized the patterns in recent security announcements (see this issue)
- restructured the project files to accommodate Drupal version-specific reviews
- implemented initial form api review focusing on '#title', '#description', and '#default_value' items
July 27
- implemented initial theme review with theme('table', ...) calls (see third item mentioned in link above)
- this provides a pattern that can be extended to other core theme functions (known to expect sanitized input)
-
the review code
- evaluates the parameters in a call to theme('table', ...)
- for each parameter that is a variable, it traverses the body of the function looking for assignments to the variables
- the traversal includes all other assignments related to that variable
- evaluates each assignment value using the secure_text_review routine
- for example, in the sample function below with theme('table', ...) call, the $xss_title and $xss_description variables used in $header assignments (multiple headers are for testing only) are resolved to the string assignments at the beginning of the function
<?php
function theme_example_table_1($form) {
$xss_title = '<a href=xss.com>xss vulnerable title</a>';
$xss_description = '<a href=xss.com>xss vulnerable description</a>';
$rows = array();
foreach (element_children($form) as $key) {
$template = &$form[$key];
$row = array();
$row[] = check_plain($template['name']);
$row[] = drupal_render($template['desc']);
$row[] = drupal_render($template['module']);
$row[] = array('data' => $template['status']['#value'], 'class' => 'status');
$row[] = array('data' => check_plain($template['status']['#value']), 'class' => 'status');
$rows[] = array('data' => $row, 'class' => $template['status']['#class']);
}
$header = array(t('Name'), t('Description'), t('Module'), t('Status'));
$header = array(t('Name'), t('Description'), t('Module'), t('Status'), $xss_title);
$header[] = array('data' => $xss_description);
$header[] = $xss_description;
$caption = $xss_title;
$output .= theme('table', $header, $rows, array(), $caption);
return $output;
}
?>August 10
-
revise logging to
- produce machine readable output (JSON format)
- utilize severity level toggles (pass, fail, unknown)
-
clean up log messages; utilize new logging format
- add constants and setting for secure_code_review_log_level
- display settings form using vertical tabs.
August 17
- catalog counts of theme() calls by theme function in D6 and D7
-
apply the code pattern to other core theme functions (known to expect sanitized input)
- item_list
Current
- apply the code pattern to other core theme functions (known to expect sanitized input)
-
revise logging to
- produce machine readable output (JSON or XML format)
- utilize some type of severity level toggles (e.g., only log errors, etc.)
Future
- after that, attempt to review custom theme functions (i.e. those defined by a contributed module) to determine assumptions about sanitized input
- from the prior step, utilize the theme review code pattern with the custom theme functions
- extend all text reviews to search the code block for assignments to the variable being reviewed
- this affects the form api review and others written last year