Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[BUG] Faulty DATETIME ticksSinceMidnight overflow check #34828

Closed
1 of 3 tasks
Marcono1234 opened this issue May 7, 2023 · 0 comments · Fixed by #34876
Closed
1 of 3 tasks

[BUG] Faulty DATETIME ticksSinceMidnight overflow check #34828

Marcono1234 opened this issue May 7, 2023 · 0 comments · Fixed by #34876
Assignees
@mkrueger
Labels
Client This issue points to a problem in the data-plane of the library. Cosmos customer-reported Issues that are reported by GitHub users external to the Azure organization. needs-team-triage Workflow: This issue needs the team to triage. question The issue doesn't require a change to the product in order to be resolved. Most issues start as that

Comments

@Marcono1234
Copy link

Marcono1234 commented May 7, 2023 •
edited by kushagraThapar
Loading

Describe the bug
The Integer.MAX_VALUE < ticksSinceMidnight check here is faulty:

azure-sdk-for-java/sdk/cosmos/azure-cosmos-encryption/src/main/java/com/azure/cosmos/encryption/implementation/mdesrc/cryptography/SqlSerializerUtil.java

Line 521 in e61b855

if (8 != decryptedValue.length || Integer.MAX_VALUE < ticksSinceMidnight) {

ticksSinceMidnight is an int, so it cannot be > Integer.MAX_VALUE. Maybe the solution here is to change ticksSinceMidnight to long (and adjust the calculation from * 10 to * 10L to avoid overflow there).

I am not familiar with this code though and am not planning to submit a pull request.

Side note: This looks suspiciously similar to the same bug reported in microsoft/mssql-jdbc#2124.

Information Checklist
Kindly make sure that you have added all the following information above and checkoff the required fields otherwise we will treat the issuer as an incomplete report

  • Bug Description Added
  • Repro Steps Added
  • Setup information Added
@github-actions github-actions bot added Client This issue points to a problem in the data-plane of the library. Cosmos customer-reported Issues that are reported by GitHub users external to the Azure organization. needs-team-triage Workflow: This issue needs the team to triage. question The issue doesn't require a change to the product in order to be resolved. Most issues start as that labels May 7, 2023
mkrueger added a commit to mkrueger/azure-sdk-for-java that referenced this issue May 10, 2023
@mkrueger
Fixed ticksSinceMidnight check.
cf4cccc 
Doesn't make sense because that is always false there is no int greater
than MAX_VALUE.
Since the byte array length is checked it only makes sense to check that
the int is positive.

Fixes Azure#34828
@mkrueger mkrueger mentioned this issue May 10, 2023
Merged
5 tasks
mkrueger added a commit to mkrueger/azure-sdk-for-java that referenced this issue May 10, 2023
@mkrueger
Fixed ticksSinceMidnight check.
67b53f9 
Casting to long should fix the issue. Added a check for negative
numbers.

Fixes Azure#34828
FabianMeiswinkel pushed a commit that referenced this issue Jun 9, 2023
@mkrueger
Fixed ticksSinceMidnight check. (#34876)
43f68e7 
Casting to long should fix the issue. Added a check for negative
numbers.

Fixes #34828
@github-actions github-actions bot locked and limited conversation to collaborators Sep 7, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@mkrueger mkrueger

Labels
Client This issue points to a problem in the data-plane of the library. Cosmos customer-reported Issues that are reported by GitHub users external to the Azure organization. needs-team-triage Workflow: This issue needs the team to triage. question The issue doesn't require a change to the product in order to be resolved. Most issues start as that
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fixed ticksSinceMidnight validation check. mkrueger/azure-sdk-for-java
2 participants
@mkrueger @Marcono1234