Is CWE-525 still relevant?#

During a code upgrade for a web application from Symfony 2.8 to 3.3 it also became time to do some basic tests with Zed Attack Proxy. While most findings were logical and easy to fix, but one was different and it started with the finding below.

Description: The AUTOCOMPLETE attribute is not disabled on an HTML FORM/INPUT element containing password type input. Passwords may be stored in browsers and retrieved.

Solution: Turn off the AUTOCOMPLETE attribute in forms or individual input elements containing password inputs by using AUTOCOMPLETE=’OFF’.

Included was the reference to CWE-525 that describes the risk and how to fix it. It leads back to the login section described in a Twig template file as below not having the proper attributes set on the form and/or input element.

<form class="navbar-form navbar-right" action="{{ path('login') }}" method="post">
    <div class="form-group">
        <input type="text" id="username" name="_username" placeholder="Email" class="form-control">
    </div>
    <div class="form-group">
        <input type="password" id="password" name="_password" placeholder="Password" class="form-control">
    </div>
    <button type="submit" class="btn btn-success">Sign in</button>
</form>

Mozilla Developer Network has an article about turning off form autocomplete for sensitive fields that browsers shouldn’t cache. Storing credit card data for example isn’t a good idea unless it is in a secured storage area. Updating the template file to contain right attributes and ZAP doesn’t complain anymore as we tell the browser not to store the password field.

<form class="navbar-form navbar-right" action="{{ path('login') }}" method="post">
    <div class="form-group">
        <input type="text" id="username" name="_username" placeholder="Email" class="form-control">
    </div>
    <div class="form-group">
        <input type="password" id="password" name="_password" placeholder="Password" class="form-control" autocomplete="off">
    </div>
    <button type="submit" class="btn btn-success">Sign in</button>
</form>

The story doesn’t end by adding the right attributes to resolve the finding, because browser makers have moved on as the attribute is being ignored for input elements of the type password for example. This as the benefit of having people store a secure password in password manager has a better risk score than people remembering and (re)using weak passwords. With this is mind we basically fixed a false positive to make the audit finding green.

For this reason, many modern browsers do not support autocomplete=”off” for login fields:

  • If a site sets autocomplete=”off” for a form, and the form includes username and password input fields, then the browser will still offer to remember this login, and if the user agrees, the browser will autofill those fields the next time the user visits the page.

  • If a site sets autocomplete=”off” for username and password input fields, then the browser will still offer to remember this login, and if the user agrees, the browser will autofill those fields the next time the user visits the page.

Here comes the problem as ZAP generates a finding that a developer has to spend time on as he needs to investigate and then has to implement a solution or explain why it isn’t worth fixing. Security scanners are handy for detecting low hanging fruit, but they shouldn’t become a time-waster with outdated rules as developers will start ignoring findings or let findings slowly starve on their backlog.