Jeff King [off-list ref] writes:

On Tue, Jul 15, 2025 at 01:28:28PM +0200, Karthik Nayak wrote:

quoted

+static int cache_ref_iterator_seek(struct ref_iterator *ref_iterator,
+				   const char *refname, unsigned int flags)
[...]
+		do {
+			int len, idx;
+			int cmp = 0;
+
+			sort_ref_dir(dir);
+
+			slash = strchr(slash, '/');
+			len = slash ? slash - refname : (int)strlen(refname);

I was looking at this code due to a nearby thread and noticed this funny
cast to int. I guess you added it to silence -Wsign-compare, but Why are
we not using a size_t in the first place?

That's an oversight from my side.

This kind of conversion can sometimes have security implications because
a very large "refname" would cause "len" to become negative (i.e., if
it's between 2GB and 4GB).

Indeed, I didn't think of that.

In this particular case it ends up cast back to a size_t via strncmp:

quoted

+			for (idx = 0; idx < dir->nr; idx++) {
+				cmp = strncmp(refname, dir->entries[idx]->name, len);
+				if (cmp <= 0)
+					break;
+			}

so we get the original value back. We'd still get truncation for a
refname value over 4GB, which would presumably give us a slightly wrong
answer. But I don't think we'd ever look outside the array.

Such sizes are probably unlikely if we are feeding filesystem paths. But
we probably should not set a bad example, and just do;

Agreed.

Thanks, I think we have to typecast `slash - refname` to size_t, but
this is the right way to do it. Thanks for the review!

Karthik