ESAPI / esapi-java-legacy

ESAPI (The OWASP Enterprise Security API) is a free, open source, web application security control library that makes it easier for programmers to write lower-risk applications.
https://owasp.org/www-project-enterprise-security-api/
Other
610 stars 368 forks source link

HTMLEntityCodec Mysteriously decodes &or #827

Closed xeno6696 closed 8 months ago

xeno6696 commented 8 months ago

Somehow the input &origin=ourprogram is translated to ∨igin=ourprogram

See discussion in #824

planetlevel commented 8 months ago

[image: image.png]

On Tue, Jan 23, 2024 at 9:17 AM Matt Seil @.***> wrote:

Somehow the input &origin=ourprogram is translated to ∨igin=ourprogram

See discussion in #824 https://github.com/ESAPI/esapi-java-legacy/issues/824

— Reply to this email directly, view it on GitHub https://github.com/ESAPI/esapi-java-legacy/issues/827, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAUUFTA4UBGWK2NRZIALYH3YP7BALAVCNFSM6AAAAABCHCJIKOVHI2DSMVQWIX3LMV43ASLTON2WKOZSGA4TMMJZGEYTGMY . You are receiving this because you are subscribed to this thread.Message ID: @.***>

-- Thanks,

--Jeff 410-707-1487

planetlevel commented 8 months ago

It's not a "v" it's a....

[image: image.png]

On Tue, Jan 23, 2024 at 10:49 AM planetlevel @.***> wrote:

[image: image.png]

On Tue, Jan 23, 2024 at 9:17 AM Matt Seil @.***> wrote:

Somehow the input &origin=ourprogram is translated to ∨igin=ourprogram

See discussion in #824 https://github.com/ESAPI/esapi-java-legacy/issues/824

— Reply to this email directly, view it on GitHub https://github.com/ESAPI/esapi-java-legacy/issues/827, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAUUFTA4UBGWK2NRZIALYH3YP7BALAVCNFSM6AAAAABCHCJIKOVHI2DSMVQWIX3LMV43ASLTON2WKOZSGA4TMMJZGEYTGMY . You are receiving this because you are subscribed to this thread.Message ID: @.***>

-- Thanks,

--Jeff 410-707-1487

-- Thanks,

--Jeff 410-707-1487

xeno6696 commented 8 months ago

I hadn't gone in to check its backing codepoint which is where my trust would be placed, but the reason that it's mysterious is that I'm not familiar with an named HTML entity for OR. Somehow when I cracked the Codec yesterday I missed line 509:

image

Hence why I thought it was mysterious.

xeno6696 commented 8 months ago

This is working as designed, not a bug.

kwwall commented 4 months ago

@xeno6696 - You closed this as 'completed' and a comment that says "This is working as designed, not a bug", but what I wanted to ask about was that you left 2 Junit tests in HTMLEntityCodecTest.java are marked as @Ignore. I understand that was more for future research, but it seems to me that we should either remote those tests or make it match the \∨ (that is, the logical OR, ∨). In general, I don't like to leave tests as ignored. I just noticed this when I ran mvn test and saw that 2 tests were skipped.

Would you prefer I just delete the tests or try to patch them up, along with a comment about how the whole safe-harbor makes it recognize the first 3 characters of '&origin=ourprogram' as the logical OR entity?

planetlevel commented 4 months ago

Have we tested modern browsers to see if they require the ; these days? If they don't, then we're stuck with the current behavior. But if they have started to require the ; it might be worth changing ESAPI to not decode unless the ; is there.

On Tue, May 28, 2024 at 5:12 PM Kevin W. Wall @.***> wrote:

@xeno6696 https://github.com/xeno6696 - You closed this as 'completed' and a comment that says "This is working as designed, not a bug", but what I wanted to ask about was that you left 2 Junit tests in HTMLEntityCodecTest.java https://github.com/ESAPI/esapi-java-legacy/blob/develop/src/test/java/org/owasp/esapi/codecs/HTMLEntityCodecTest.java#L53-L70 are marked as @Ignore. I understand that was more for future research, but it seems to me that we should either remote those tests or make it match the ∨ (that is, the logical OR, ∨). In general, I don't like to leave tests as ignored. I just noticed this when I ran mvn test and saw that 2 tests were skipped.

Would you prefer I just delete the tests or try to patch them up, along with a comment about how the whole safe-harbor makes it recognize the first 3 characters of '&origin=ourprogram' as the logical OR entity?

— Reply to this email directly, view it on GitHub https://github.com/ESAPI/esapi-java-legacy/issues/827#issuecomment-2136117369, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAUUFTC463CV4LAKCGRFXE3ZETXMFAVCNFSM6AAAAABCHCJIKOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCMZWGEYTOMZWHE . You are receiving this because you commented.Message ID: @.***>

-- Thanks,

--Jeff 410-707-1487

xeno6696 commented 4 months ago

@planetlevel both render in chrome and safari, that’s enough for me.

xeno6696 commented 4 months ago

Feel free to delete the tests.

kwwall commented 4 months ago

@xeno6696 wrote:

@planetlevel both render in chrome and safari, that’s enough for me.

I assume you were testing both browsers on MacOS then. On Linux Mint 21.3 neither the latest version of Firefox or Chrome treats &or as entity encoding without the trailing ';'. I find it odd that Chrome on MacOS would be different. Or maybe you tested with a different entity-encoding.

planetlevel commented 4 months ago

Weird.  &or doesn’t render for me in Safari or Chrome on Mac.  But ∨ renders fine.  But &lt does render just like < in both.To really test this, you’d need to test all the various contexts.  Quoted and unquoted attributes, URLs, in JavaScript, etc…At least what we do now is principled. :-)—JeffOn May 28, 2024, at 8:46 PM, Kevin W. Wall @.***> wrote: @xeno6696 wrote:

@planetlevel both render in chrome and safari, that’s enough for me.

I assume you were testing both browsers on MacOS then. On Linux Mint 21.3 neither the latest version of Firefox or Chrome treats &or as entity encoding without the trailing ';'. I find it odd that Chrome on MacOS would be different. Or maybe you tested with a different entity-encoding.

—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you were mentioned.Message ID: @.***>

xeno6696 commented 4 months ago

So yeah, my test was just:&
&ampAnd yeah, I just assumed from there.  I wouldn’t expect different behavior but what we have now is a sane default.  I can run a more comprehensive test tomorrow.Sent from my iPhoneOn May 28, 2024, at 20:33, Jeff Williams @.> wrote: Weird.  &or doesn’t render for me in Safari or Chrome on Mac.  But ∨ renders fine.  But &lt does render just like < in both.To really test this, you’d need to test all the various contexts.  Quoted and unquoted attributes, URLs, in JavaScript, etc…At least what we do now is principled. :-)—JeffOn May 28, 2024, at 8:46 PM, Kevin W. Wall @.> wrote:

@xeno6696 wrote:

@planetlevel both render in chrome and safari, that’s enough for me.

I assume you were testing both browsers on MacOS then. On Linux Mint 21.3 neither the latest version of Firefox or Chrome treats &or as entity encoding without the trailing ';'. I find it odd that Chrome on MacOS would be different. Or maybe you tested with a different entity-encoding.

—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you were mentioned.Message ID: @.***>

—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you were mentioned.Message ID: @.***>