Skip to content

[C++][Fuzzing] Fix bug discovered by fuzzing #45180

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.

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

Closed
zeroshade opened this issue Jan 6, 2025 · 2 comments
Closed

[C++][Fuzzing] Fix bug discovered by fuzzing #45180

zeroshade opened this issue Jan 6, 2025 · 2 comments

Comments

@zeroshade
Copy link
Member

Describe the bug, including details regarding any error messages, version, and platform.

Decimal32 and Decimal64 negation methods allowed for overflows if the value was INT32_MIN or INT64_MIN which were discovered by OSS-Fuzz. This is easily fixed by using arrow::internal::SafeSignedNegate.

Component(s)

C++, Continuous Integration

pitrou pushed a commit that referenced this issue Jan 13, 2025
### Rationale for this change
If the value for Decimal32 or Decimal64 is `INT32_MIN` or `INT64_MIN` respectively, then UBSAN reports an issue when calling Negate on them due to overflow. 

### What changes are included in this PR?
Have the `Negate` methods of Decimal32 and Decimal64 use `arrow::internal::SafeSignedNegate`.

### Are these changes tested?
Unit tests were added for both cases which were able to reproduce the problem when UBSAN was on without the fix.

### Are there any user-facing changes?
No.

* OSS-Fuzz issue: https://issues.oss-fuzz.com/issues/371239168

* GitHub Issue: #45180

Authored-by: Matt Topol <[email protected]>
Signed-off-by: Antoine Pitrou <[email protected]>
@pitrou pitrou added this to the 20.0.0 milestone Jan 13, 2025
@pitrou
Copy link
Member

pitrou commented Jan 13, 2025

Issue resolved by pull request 45181
#45181

@pitrou pitrou closed this as completed Jan 13, 2025
@raulcd raulcd modified the milestones: 20.0.0, 19.0.1 Jan 29, 2025
@raulcd
Copy link
Member

raulcd commented Jan 29, 2025

@amoeba FYI I've added the 19.0.1 milestone as per backport-candidate label.

amoeba pushed a commit that referenced this issue Jan 31, 2025
### Rationale for this change
If the value for Decimal32 or Decimal64 is `INT32_MIN` or `INT64_MIN` respectively, then UBSAN reports an issue when calling Negate on them due to overflow. 

### What changes are included in this PR?
Have the `Negate` methods of Decimal32 and Decimal64 use `arrow::internal::SafeSignedNegate`.

### Are these changes tested?
Unit tests were added for both cases which were able to reproduce the problem when UBSAN was on without the fix.

### Are there any user-facing changes?
No.

* OSS-Fuzz issue: https://issues.oss-fuzz.com/issues/371239168

* GitHub Issue: #45180

Authored-by: Matt Topol <[email protected]>
Signed-off-by: Antoine Pitrou <[email protected]>
amoeba pushed a commit that referenced this issue Jan 31, 2025
### Rationale for this change
If the value for Decimal32 or Decimal64 is `INT32_MIN` or `INT64_MIN` respectively, then UBSAN reports an issue when calling Negate on them due to overflow. 

### What changes are included in this PR?
Have the `Negate` methods of Decimal32 and Decimal64 use `arrow::internal::SafeSignedNegate`.

### Are these changes tested?
Unit tests were added for both cases which were able to reproduce the problem when UBSAN was on without the fix.

### Are there any user-facing changes?
No.

* OSS-Fuzz issue: https://issues.oss-fuzz.com/issues/371239168

* GitHub Issue: #45180

Authored-by: Matt Topol <[email protected]>
Signed-off-by: Antoine Pitrou <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants