lazysodium-java icon indicating copy to clipboard operation
lazysodium-java copied to clipboard

Prevent memory access errors in many native calls by adding checks

Open timmc opened this issue 5 years ago • 3 comments

This is not comprehensive coverage, but should take care of a number of unguarded calls. It comes out of issue #81 in which a call to cryptoSecretStreamPush with an incorrectly sized array lead to a VM crash.

  • Add array checkers to BaseChecker
  • Simplify some existing checks by moving the throw into the check method; add reporting of which variable failed a check
  • Add checkers for SecretStream
  • Split PwHash.Checker.checkAll into individual check calls to reduce chance of positional argument errors

timmc avatar Feb 08 '21 02:02 timmc

Tests fail on a call to sodiumPad, but I think the test may be wrong: https://github.com/terl/lazysodium-java/blob/f7f0025/src/test/java/com/goterl/lazycode/lazysodium/PaddingTest.java#L19-L23

Shouldn't the max buffer length always be less than or equal to the length of the buffer?

timmc avatar Feb 08 '21 23:02 timmc

@timmc I haven't checked in a while, but I think sodiumPad is still broken, details are in #85

ionspin avatar Feb 21 '21 19:02 ionspin

For reference, the ./gradlew test currently produces this test failure:

java.lang.IllegalArgumentException: Provided buffer array length is larger than array
	at com.goterl.lazysodium.utils.BaseChecker.checkArrayLength(BaseChecker.java:59)
	at com.goterl.lazysodium.utils.BaseChecker.checkArrayLength(BaseChecker.java:50)
	at com.goterl.lazysodium.LazySodium.sodiumPad(LazySodium.java:186)
	at com.goterl.lazysodium.PaddingTest.pad(PaddingTest.java:22)

timmc avatar Apr 16 '21 02:04 timmc