[
https://issues.apache.org/jira/browse/FTPSERVER-485?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16445919#comment-16445919
]
Emmanuel Lecharny commented on FTPSERVER-485:
---------------------------------------------
I would say that it does not harm to strengthen the code to get rid f such
remote weakness. I'm pretty sure this issue has been found using some tool, so
expect to have more than one ticket to address.
In any case, the proposed patch does not solve the problem : it's easy to write
some code that makes it possible to leak some information from it, as switching
boolean tags is way faster than comparing chars from arrays. Add to that JIT
might come to play here, making it even harder to have a stable and non leaking
match function.
I would go with a function like this one :
{code:java}
private static boolean compare( String source, String target )
{
if ( ( source == null ) || ( target == null ) )
{
return source == target;
}
if ( source.length() != target.length() )
{
return false;
}
int result = 0;
for ( int i = 0; i < target.length(); i++ )
{
result |= source.charAt(i) ^ target.charAt( i );
}
return result == 0;
}
{code}
> Timing Side Channel PasswordEncryptor
> -------------------------------------
>
> Key: FTPSERVER-485
> URL: https://issues.apache.org/jira/browse/FTPSERVER-485
> Project: FtpServer
> Issue Type: Bug
> Components: Core
> Affects Versions: 1.1.1
> Environment: tested on macOS High Sierra 10.13.4, but it is not
> relevant
> Reporter: Yannic Noller
> Assignee: Jonathan Valliere
> Priority: Major
> Labels: easyfix, pull-request-available
> Fix For: 1.1.2
>
> Original Estimate: 24h
> Remaining Estimate: 24h
>
> Dear Apache FTPServer developers,
> We have found a timing side-channel in class
> org.apache.ftpserver.usermanager.ClearTextPasswordEncryptor, method "public
> boolean matches(String passwordToCheck, String storedPassword)". This is due
> to the use of String.equals for comparison which returns as soon as a
> character does not match. This represents a timing side channel, which could
> be used by a potential attacker to obtain knowledge about the hidden secret
> password.
> Do you agree with our findings?
> A similar issue is present in method "matches" from classes
> org.apache.ftpserver.usermanager.Md5PasswordEncryptor and
> org.apache.ftpserver.usermanager.SaltedPasswordEncryptor.
> We found these classes in the latest version of your git repo:
> https://git-wip-us.apache.org/repos/asf?p=mina-ftpserver.git;a=summary
> The problem can be fixed easily by using the following safe version for
> String comparison in all three methods:
> public boolean isEqual_safe(String a, String b) {
> char a_value[] = a.toCharArray();
> char b_value[] = b.toCharArray();
> boolean unused;
> boolean matches = true;
> for (int i = 0; i < a_value.length; i++) {
> if (i < b_value.length) {
> if (a_value[i] != b_value[i]) {
> matches = false;
> } else {
> unused = true;
> }
> } else {
> unused = false;
> unused = true;
> }
> }
> return matches;
> }
> Do you agree with our patch proposal?
> Please feel free to contact us for further clarification! You can reach us by
> the following email address:
> [email protected]
> Best regards,
> Yannic Noller
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
