Skip to content

patch _imagingcms.c: use strncpy #153

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

Open
vrunoa wants to merge 2 commits into
base: simd/master
Choose a base branch
from
Open

patch _imagingcms.c: use strncpy #153

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a8efbdd
patch _imagingcms.c: use strncpy
balassia-prenuvo Aug 8, 2025
c8a209a
add test as upstream
balassia-prenuvo Aug 8, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions Tests/test_imagecms.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -626,6 +626,9 @@ def test_constants_deprecation():
with pytest.warns(DeprecationWarning):
assert getattr(ImageCms, prefix + name) == enum[name]

def test_long_modes() -> None:
p = ImageCms.getOpenProfile("Tests/icc/sGrey-v2-nano.icc")
ImageCms.buildTransform(p, p, "ABCDEFGHI", "ABCDEFGHI")

@pytest.mark.parametrize("mode", ("RGB", "RGBA", "RGBX"))
def test_rgb_lab(mode):
Expand Down
4 changes: 2 additions & 2 deletions src/_imagingcms.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -201,8 +201,8 @@ cms_transform_new(cmsHTRANSFORM transform, char *mode_in, char *mode_out) {

self->transform = transform;

strcpy(self->mode_in, mode_in);
strcpy(self->mode_out, mode_out);
strncpy(self->mode_in, mode_in, 8);
strncpy(self->mode_out, mode_out, 8);
Comment on lines +204 to +205
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Aug 8, 2025

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

strncpy here can leave mode strings unterminated; ensure explicit NUL-termination

strncpy does not NUL-terminate when the source length ≥ n. For 8-char modes like "RGBA;16B", this yields an unterminated buffer and PyUnicode_FromString(self->mode_in/out) may read past the array, causing OOB reads/UB. Copy up to sizeof(buf)-1 and force-terminate.

Apply this minimal, safe fix:

-    strncpy(self->mode_in, mode_in, 8);
-    strncpy(self->mode_out, mode_out, 8);
+    strncpy(self->mode_in, mode_in, sizeof(self->mode_in) - 1);
+    self->mode_in[sizeof(self->mode_in) - 1] = '\0';
+    strncpy(self->mode_out, mode_out, sizeof(self->mode_out) - 1);
+    self->mode_out[sizeof(self->mode_out) - 1] = '\0';

Optional follow-ups (pick one, if you want to avoid truncating 8-char modes):

  • Keep arrays as-is and change getters to not rely on a trailing NUL:
    • inputMode getter: return PyUnicode_FromStringAndSize(self->mode_in, strnlen(self->mode_in, sizeof(self->mode_in)));
    • outputMode getter: same pattern.
  • Or increase buffers to 9 and keep the above termination pattern to preserve all 8 chars plus '\0' (verify ABI considerations; these structs are internal, but confirm downstream assumptions).

🏁 Script executed:

#!/bin/bash
set -e
echo "---- struct definition around mode_in/mode_out ----"
rg -n "mode_in" -A3 -B3 src/_imagingcms.c || true
rg -n "mode_out" -A3 -B3 src/_imagingcms.c || true

echo "---- strncpy usages ----"
rg -n -C2 "strncpy" src/_imagingcms.c || true

echo "---- Getter functions using mode_in/mode_out ----"
rg -n "PyUnicode_FromString" -C3 src/_imagingcms.c || true

Length of output: 3227

Ensure mode_in/mode_out are always NUL-terminated

strncpy(self->mode_in, mode_in, 8) and likewise for mode_out won’t append '\0' if the source is ≥ 8 bytes (e.g. "RGBA;16B"). Since the getters call PyUnicode_FromString(), a missing terminator can lead to OOB reads and undefined behavior.

• File: src/_imagingcms.c
Lines 204–205 (inside cms_transform_new): replace the two strncpy calls with:

-    strncpy(self->mode_in, mode_in, 8);
-    strncpy(self->mode_out, mode_out, 8);
+    strncpy(self->mode_in, mode_in, sizeof(self->mode_in) - 1);
+    self->mode_in[sizeof(self->mode_in) - 1] = '\0';
+    strncpy(self->mode_out, mode_out, sizeof(self->mode_out) - 1);
+    self->mode_out[sizeof(self->mode_out) - 1] = '\0';

Optional follow-ups (choose one):

  • Change the getters to use PyUnicode_FromStringAndSize() with strnlen() to avoid relying on a terminator.
  • Increase the buffer sizes to 9 to allow full 8-char modes plus '\0' (verify ABI impact).
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
strncpy(self->mode_in, mode_in, 8);
strncpy(self->mode_out, mode_out, 8);
/* ensure mode_in/mode_out are always NUL-terminated */
strncpy(self->mode_in, mode_in, sizeof(self->mode_in) - 1);
self->mode_in[sizeof(self->mode_in) - 1] = '\0';
strncpy(self->mode_out, mode_out, sizeof(self->mode_out) - 1);
self->mode_out[sizeof(self->mode_out) - 1] = '\0';
🤖 Prompt for AI Agents 
In src/_imagingcms.c at lines 204-205 inside cms_transform_new, the strncpy
calls copying mode_in and mode_out do not guarantee NUL-termination if the
source strings are 8 or more characters, risking out-of-bounds reads. Replace
these strncpy calls by copying up to 7 characters and explicitly setting the 8th
byte to '\0' to ensure proper NUL-termination of the mode_in and mode_out
buffers.


return (PyObject *)self;
}
Expand Down