Changing from 'break;' to 'continue;' is the most straightforward
fix, but I'd recommend adding a comment as to WHY you are doing
this, so that later on, someone doesn't come back to "optimize"
the code and change it back to 'break'. I'd just put something like
this at the beginning of the 'for' loop:
// Must compare *ALL* bytes of the hash, otherwise a
// side-channel timing attack is possible.
And I agree, very low probability, although this sort of thing is
not unheard of. It just takes a monstrous # of requests (sometimes
millions) to get a good statistical average to distinguish things
at the microsec range. But if it's this easy to fix, why not???
-kevin
---
Kevin W. Wall Qwest Information Technology, Inc.
[email protected] Phone: 614.215.4788
"It is practically impossible to teach good programming to students
that have had a prior exposure to BASIC: as potential programmers
they are mentally mutilated beyond hope of regeneration"
- Edsger Dijkstra, How do we tell truths that matter?
http://www.cs.utexas.edu/~EWD/transcriptions/EWD04xx/EWD498.html
> -----Original Message-----
> From: Leonardo Uribe (JIRA) [mailto:[email protected]]
> Sent: Thursday, September 30, 2010 2:30 PM
> To: [email protected]
> Subject: [jira] Commented: (MYFACES-2934) Side-channel timing attack in
> StateUtils class may still allow padding oracle attack
>
>
> [ https://issues.apache.org/jira/browse/MYFACES-
> 2934?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-
> tabpanel&focusedCommentId=12916595#action_12916595 ]
>
> Leonardo Uribe commented on MYFACES-2934:
> -----------------------------------------
>
> I have checked it and well, the probability to be succesful using that type
> of attack is very, very, very, very, very low, because the comparison done
> in this case takes very few time compared with other tasks done by myfaces
> itself on a single request.
>
> Anyway, I would like to know how to fix it, maybe replace the line "break"
> with "continue" should do the job. If that so, I can commit the code on all
> myfaces branches.
>
>
> > Side-channel timing attack in StateUtils class may still allow padding
> oracle attack
> > -------------------------------------------------------------------------
> -----------
> >
> > Key: MYFACES-2934
> > URL: https://issues.apache.org/jira/browse/MYFACES-2934
> > Project: MyFaces Core
> > Issue Type: Bug
> > Affects Versions: 1.2.9
> > Environment: All using MyFaces 1.2.9
> > Reporter: Kevin W. Wall
> > Original Estimate: 48h
> > Remaining Estimate: 48h
> >
> > [FYI: I'm the person who fixed the padding oracle attack in ESAPI 2.0-rc#
> crypto which is why I spotted this.]
> > I did a quick code inspection of encrypt() / decrypt() methods in
> org.apache.myfaces.shared_impl.util.StateUtils as it relates to the fix for
> MYFACES-2749. Most everything is done correct (MAC is over IV+ciphertext
> and checked before decryption), but I noticed a subtle flaw that, at least
> in theory (or enough data gathering and statistical analysis), that opens a
> side-channel timing attack that might be still be used as a oracle in a
> padded oracle attack such as described by Duong and Rizzo.
> > The problem is in the 'for' loop at lines 471-478 in StateUtils.java. You
> need to compare ALWAYS compare ALL the bytes in the MAC to ensure a timing
> side-channel attack cannot be used to as an oracle in the padding oracle
> attack.
> > Contact me at [email protected] if you need more info or want to see
> how it was fixed in OWASP ESAPI.
>
> --
> This message is automatically generated by JIRA.
> -
> You can reply to this email to add a comment to the issue online.
This communication is the property of Qwest and may contain confidential or
privileged information. Unauthorized use of this communication is strictly
prohibited and may be unlawful. If you have received this communication
in error, please immediately notify the sender by reply e-mail and destroy
all copies of the communication and any attachments.
