Junio C Hamano <gitster@xxxxxxxxx> writes:
> Johannes Schindelin <johannes.schindelin@xxxxxx> writes:
> ...
>> If the buffer does *not* contain an empty line, the fsck code runs the
>> danger of looking beyond the allocated memory because it uses
>> functions that assume NUL-terminated strings, while the buffer passed
>> to the fsck code is a counted string.
>> that already, but not all of them.
> ...
> I do not think you necessarily need a NUL. As you said, your input
> is a counted string, so you know the length of the buffer. And you
> are verifying line by line. As long as you make sure the buffer
> ends with "\n" (instead of saying "it has "\n\n" somewhere),
> updating the existing code that does ...
> to do this instead: ...
> shouldn't be rocket science, no?
Here is to illustrate what I meant by the above.
I did only the "tag" side (this is on 'maint'), because I did not
want to conflict with a larger change to fsck you have on 'pu'.
I made it use an updated version of require_end_of_header(), which I
named prescan_headers(), as the new sanity check does not require
\n\n as the only end of header, and also the sanity check (both old
and new) also checks for NUL.
I didn't assess how much more involved to make fsck-commit-buffer to
use prescan-headers (and retire require-end-of-header), though.
fsck.c | 39 ++++++++++++++++++++++++++++++++++-----
1 file changed, 34 insertions(+), 5 deletions(-)
diff --git a/fsck.c b/fsck.c
index 10bcb65..7d2d07d 100644
--- a/fsck.c
+++ b/fsck.c
@@ -261,6 +261,29 @@ static int require_end_of_header(const void *data, unsigned long size,
return error_func(obj, FSCK_ERROR, "unterminated header");
}
+static int prescan_headers(const void *data, unsigned long size,
+ struct object *obj, fsck_error error_func)
+{
+ const char *buffer = (const char *)data;
+ unsigned long i;
+
+ for (i = 0; i < size; i++) {
+ switch (buffer[i]) {
+ case '\0':
+ return error_func(obj, FSCK_ERROR,
+ "unterminated header: NUL at offset %d", i);
+ case '\n':
+ if (i + 1 < size && buffer[i + 1] == '\n')
+ return 0; /* end of header */
+ }
+ }
+
+ /* no blank (i.e. headers and nothing else */
+ if (size && buffer[size - 1] == '\n')
+ return 0;
+ return error_func(obj, FSCK_ERROR, "unterminated header");
+}
+
static int fsck_ident(const char **ident, struct object *obj, fsck_error error_func)
{
char *end;
@@ -365,6 +388,7 @@ static int fsck_tag_buffer(struct tag *tag, const char *data,
unsigned char sha1[20];
int ret = 0;
const char *buffer;
+ const char *end_buffer;
char *to_free = NULL, *eol;
struct strbuf sb = STRBUF_INIT;
@@ -387,10 +411,12 @@ static int fsck_tag_buffer(struct tag *tag, const char *data,
}
}
- if (require_end_of_header(buffer, size, &tag->object, error_func))
+ end_buffer = buffer + size;
+ if (prescan_headers(buffer, size, &tag->object, error_func))
goto done;
- if (!skip_prefix(buffer, "object ", &buffer)) {
+ if (end_buffer <= buffer ||
+ !skip_prefix(buffer, "object ", &buffer)) {
ret = error_func(&tag->object, FSCK_ERROR, "invalid format - expected 'object' line");
goto done;
}
@@ -400,7 +426,8 @@ static int fsck_tag_buffer(struct tag *tag, const char *data,
}
buffer += 41;
- if (!skip_prefix(buffer, "type ", &buffer)) {
+ if (end_buffer <= buffer ||
+ !skip_prefix(buffer, "type ", &buffer)) {
ret = error_func(&tag->object, FSCK_ERROR, "invalid format - expected 'type' line");
goto done;
}
@@ -415,7 +442,8 @@ static int fsck_tag_buffer(struct tag *tag, const char *data,
goto done;
buffer = eol + 1;
- if (!skip_prefix(buffer, "tag ", &buffer)) {
+ if (end_buffer <= buffer ||
+ !skip_prefix(buffer, "tag ", &buffer)) {
ret = error_func(&tag->object, FSCK_ERROR, "invalid format - expected 'tag' line");
goto done;
}
@@ -430,7 +458,8 @@ static int fsck_tag_buffer(struct tag *tag, const char *data,
(int)(eol - buffer), buffer);
buffer = eol + 1;
- if (!skip_prefix(buffer, "tagger ", &buffer))
+ if (end_buffer <= buffer ||
+ !skip_prefix(buffer, "tagger ", &buffer))
/* early tags do not contain 'tagger' lines; warn only */
error_func(&tag->object, FSCK_WARN, "invalid format - expected 'tagger' line");
else
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html