Skip to content

Fix unsigned difference expression compared to zero#2101

Merged
ranshid merged 1 commit intovalkey-io:unstablefrom
odaysec:patch-1
May 26, 2025
Merged

Fix unsigned difference expression compared to zero#2101
ranshid merged 1 commit intovalkey-io:unstablefrom
odaysec:patch-1

Conversation

@odaysec
Copy link
Copy Markdown
Contributor

@odaysec odaysec commented May 19, 2025

if (ln->prev != NULL && (prev = listNodeValue(ln->prev)) && prev->size - prev->used > 0 &&

Fix the issue need to ensure that the subtraction prev->size - prev->used does not underflow. This can be achieved by explicitly checking that prev->used is less than prev->size before performing the subtraction. This approach avoids relying on unsigned arithmetic and ensures the logic is clear and robust.

The specific changes are:

  1. Replace the condition prev->size - prev->used > 0 with prev->used < prev->size.
  2. This change ensures that the logic checks whether there is remaining space in the buffer without risking underflow.

References
INT02-C. Understand integer conversion rules
CWE-191


Signed-off-by: Zeroday BYTE <github@zerodaysec.org>
@Fusl
Copy link
Copy Markdown
Contributor

Fusl commented May 20, 2025

INT02-C. Understand integer conversion rules

That link is currently just returning Down for Maintenance, here is an archived copy

Copy link
Copy Markdown
Member

@ranshid ranshid left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. rerun the jobs as it seem to fail on github runner issue

@codecov
Copy link
Copy Markdown

codecov bot commented May 26, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 71.23%. Comparing base (daea05b) to head (0b8147c).
Report is 21 commits behind head on unstable.

Additional details and impacted files
@@             Coverage Diff              @@
##           unstable    #2101      +/-   ##
============================================
- Coverage     71.25%   71.23%   -0.03%     
============================================
  Files           122      122              
  Lines         66026    66026              
============================================
- Hits          47050    47031      -19     
- Misses        18976    18995      +19     
Files with missing lines Coverage Δ
src/networking.c 87.40% <100.00%> (+0.21%) ⬆️

... and 12 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@ranshid ranshid merged commit 374718b into valkey-io:unstable May 26, 2025
51 checks passed
shanwan1 pushed a commit to shanwan1/valkey that referenced this pull request Jun 13, 2025
https://github.com/valkey-io/valkey/blob/daea05b1e26db29bfd1c033e27f9d519a2f8ccbb/src/networking.c#L886-L886

Fix the issue need to ensure that the subtraction `prev->size -
prev->used` does not underflow. This can be achieved by explicitly
checking that `prev->used` is less than `prev->size` before performing
the subtraction. This approach avoids relying on unsigned arithmetic and
ensures the logic is clear and robust.

The specific changes are:
1. Replace the condition `prev->size - prev->used > 0` with `prev->used
< prev->size`.
2. This change ensures that the logic checks whether there is remaining
space in the buffer without risking underflow.

**References**
[INT02-C. Understand integer conversion
rules](https://wiki.sei.cmu.edu/confluence/display/c/INT02-C.+Understand+integer+conversion+rules)
[CWE-191](https://cwe.mitre.org/data/definitions/191.html)

---

Signed-off-by: Zeroday BYTE <github@zerodaysec.org>
Signed-off-by: shanwan1 <shanwan1@intel.com>
@odaysec
Copy link
Copy Markdown
Contributor Author

odaysec commented Jul 16, 2025

Hi @Fusl @lipzhu @ranshid, Thank you for the advisory update!

Would it be possible to include my Github handle github:username for proper credit attribution in the advisories GHSA-xhp4-6g9v-4xvj

Thank you in advance!

@yizhao1
Copy link
Copy Markdown

yizhao1 commented Aug 4, 2025

Hello, this patch is not yet merged into 8.1 branch. Does it need to be merged into 8.1 branch? Thanks.

@zuiderkwast zuiderkwast moved this to To be backported in Valkey 7.2 Sep 23, 2025
@zuiderkwast zuiderkwast moved this to To be backported in Valkey 8.0 Sep 23, 2025
@zuiderkwast zuiderkwast moved this to To be backported in Valkey 8.1 Sep 23, 2025
@ranshid ranshid moved this from To be backported to In Progress in Valkey 8.1 Sep 30, 2025
ranshid pushed a commit to ranshid/valkey that referenced this pull request Sep 30, 2025
https://github.com/valkey-io/valkey/blob/daea05b1e26db29bfd1c033e27f9d519a2f8ccbb/src/networking.c#L886-L886

Fix the issue need to ensure that the subtraction `prev->size -
prev->used` does not underflow. This can be achieved by explicitly
checking that `prev->used` is less than `prev->size` before performing
the subtraction. This approach avoids relying on unsigned arithmetic and
ensures the logic is clear and robust.

The specific changes are:
1. Replace the condition `prev->size - prev->used > 0` with `prev->used
< prev->size`.
2. This change ensures that the logic checks whether there is remaining
space in the buffer without risking underflow.

**References**
[INT02-C. Understand integer conversion
rules](https://wiki.sei.cmu.edu/confluence/display/c/INT02-C.+Understand+integer+conversion+rules)
[CWE-191](https://cwe.mitre.org/data/definitions/191.html)


---

Signed-off-by: Zeroday BYTE <github@zerodaysec.org>
ranshid pushed a commit to ranshid/valkey that referenced this pull request Sep 30, 2025
https://github.com/valkey-io/valkey/blob/daea05b1e26db29bfd1c033e27f9d519a2f8ccbb/src/networking.c#L886-L886

Fix the issue need to ensure that the subtraction `prev->size -
prev->used` does not underflow. This can be achieved by explicitly
checking that `prev->used` is less than `prev->size` before performing
the subtraction. This approach avoids relying on unsigned arithmetic and
ensures the logic is clear and robust.

The specific changes are:
1. Replace the condition `prev->size - prev->used > 0` with `prev->used
< prev->size`.
2. This change ensures that the logic checks whether there is remaining
space in the buffer without risking underflow.

**References**
[INT02-C. Understand integer conversion
rules](https://wiki.sei.cmu.edu/confluence/display/c/INT02-C.+Understand+integer+conversion+rules)
[CWE-191](https://cwe.mitre.org/data/definitions/191.html)


---

Signed-off-by: Zeroday BYTE <github@zerodaysec.org>
@ranshid ranshid moved this from In Progress to 8.1.4 in Valkey 8.1 Sep 30, 2025
@ranshid ranshid moved this from 8.1.4 to To be backported in Valkey 8.1 Sep 30, 2025
@zuiderkwast zuiderkwast moved this from To be backported to 8.0.6 (to be released) in Valkey 8.0 Sep 30, 2025
zuiderkwast pushed a commit to zuiderkwast/valkey that referenced this pull request Sep 30, 2025
https://github.com/valkey-io/valkey/blob/daea05b1e26db29bfd1c033e27f9d519a2f8ccbb/src/networking.c#L886-L886

Fix the issue need to ensure that the subtraction `prev->size -
prev->used` does not underflow. This can be achieved by explicitly
checking that `prev->used` is less than `prev->size` before performing
the subtraction. This approach avoids relying on unsigned arithmetic and
ensures the logic is clear and robust.

The specific changes are:
1. Replace the condition `prev->size - prev->used > 0` with `prev->used
< prev->size`.
2. This change ensures that the logic checks whether there is remaining
space in the buffer without risking underflow.

**References**
[INT02-C. Understand integer conversion
rules](https://wiki.sei.cmu.edu/confluence/display/c/INT02-C.+Understand+integer+conversion+rules)
[CWE-191](https://cwe.mitre.org/data/definitions/191.html)


---

Signed-off-by: Zeroday BYTE <github@zerodaysec.org>
@zuiderkwast zuiderkwast moved this from To be backported to 7.2.11 (to be released) in Valkey 7.2 Sep 30, 2025
zuiderkwast pushed a commit to zuiderkwast/valkey that referenced this pull request Sep 30, 2025
https://github.com/valkey-io/valkey/blob/daea05b1e26db29bfd1c033e27f9d519a2f8ccbb/src/networking.c#L886-L886

Fix the issue need to ensure that the subtraction `prev->size -
prev->used` does not underflow. This can be achieved by explicitly
checking that `prev->used` is less than `prev->size` before performing
the subtraction. This approach avoids relying on unsigned arithmetic and
ensures the logic is clear and robust.

The specific changes are:
1. Replace the condition `prev->size - prev->used > 0` with `prev->used
< prev->size`.
2. This change ensures that the logic checks whether there is remaining
space in the buffer without risking underflow.

**References**
[INT02-C. Understand integer conversion
rules](https://wiki.sei.cmu.edu/confluence/display/c/INT02-C.+Understand+integer+conversion+rules)
[CWE-191](https://cwe.mitre.org/data/definitions/191.html)

---

Signed-off-by: Zeroday BYTE <github@zerodaysec.org>
zuiderkwast pushed a commit that referenced this pull request Oct 1, 2025
https://github.com/valkey-io/valkey/blob/daea05b1e26db29bfd1c033e27f9d519a2f8ccbb/src/networking.c#L886-L886

Fix the issue need to ensure that the subtraction `prev->size -
prev->used` does not underflow. This can be achieved by explicitly
checking that `prev->used` is less than `prev->size` before performing
the subtraction. This approach avoids relying on unsigned arithmetic and
ensures the logic is clear and robust.

The specific changes are:
1. Replace the condition `prev->size - prev->used > 0` with `prev->used
< prev->size`.
2. This change ensures that the logic checks whether there is remaining
space in the buffer without risking underflow.

**References**
[INT02-C. Understand integer conversion
rules](https://wiki.sei.cmu.edu/confluence/display/c/INT02-C.+Understand+integer+conversion+rules)
[CWE-191](https://cwe.mitre.org/data/definitions/191.html)

---

Signed-off-by: Zeroday BYTE <github@zerodaysec.org>
zuiderkwast pushed a commit that referenced this pull request Oct 1, 2025
https://github.com/valkey-io/valkey/blob/daea05b1e26db29bfd1c033e27f9d519a2f8ccbb/src/networking.c#L886-L886

Fix the issue need to ensure that the subtraction `prev->size -
prev->used` does not underflow. This can be achieved by explicitly
checking that `prev->used` is less than `prev->size` before performing
the subtraction. This approach avoids relying on unsigned arithmetic and
ensures the logic is clear and robust.

The specific changes are:
1. Replace the condition `prev->size - prev->used > 0` with `prev->used
< prev->size`.
2. This change ensures that the logic checks whether there is remaining
space in the buffer without risking underflow.

**References**
[INT02-C. Understand integer conversion
rules](https://wiki.sei.cmu.edu/confluence/display/c/INT02-C.+Understand+integer+conversion+rules)
[CWE-191](https://cwe.mitre.org/data/definitions/191.html)


---

Signed-off-by: Zeroday BYTE <github@zerodaysec.org>
zuiderkwast pushed a commit that referenced this pull request Oct 1, 2025
https://github.com/valkey-io/valkey/blob/daea05b1e26db29bfd1c033e27f9d519a2f8ccbb/src/networking.c#L886-L886

Fix the issue need to ensure that the subtraction `prev->size -
prev->used` does not underflow. This can be achieved by explicitly
checking that `prev->used` is less than `prev->size` before performing
the subtraction. This approach avoids relying on unsigned arithmetic and
ensures the logic is clear and robust.

The specific changes are:
1. Replace the condition `prev->size - prev->used > 0` with `prev->used
< prev->size`.
2. This change ensures that the logic checks whether there is remaining
space in the buffer without risking underflow.

**References**
[INT02-C. Understand integer conversion
rules](https://wiki.sei.cmu.edu/confluence/display/c/INT02-C.+Understand+integer+conversion+rules)
[CWE-191](https://cwe.mitre.org/data/definitions/191.html)


---

Signed-off-by: Zeroday BYTE <github@zerodaysec.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: 7.2.11
Status: 8.0.6
Status: 8.1.4

Development

Successfully merging this pull request may close these issues.

6 participants