From: Junio C Hamano <gitster@pobox.com>
To: Johannes Schindelin <johannes.schindelin@gmx.de>
Cc: Wolfgang Denk <wd@denx.de>, git@vger.kernel.org
Subject: Re: git error in tag ...: unterminated header
Date: Thu, 25 Jun 2015 15:29:45 -0700 [thread overview]
Message-ID: <xmqqbng3wheu.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <xmqqoak3wkkq.fsf@gitster.dls.corp.google.com> (Junio C. Hamano's message of "Thu, 25 Jun 2015 14:21:25 -0700")
Junio C Hamano <gitster@pobox.com> writes:
> Johannes Schindelin <johannes.schindelin@gmx.de> 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
next prev parent reply other threads:[~2015-06-25 22:29 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-06-25 15:51 git error in tag ...: unterminated header Wolfgang Denk
2015-06-25 17:32 ` Junio C Hamano
2015-06-25 20:13 ` Wolfgang Denk
2015-06-25 20:24 ` Junio C Hamano
2015-06-25 21:07 ` Johannes Schindelin
2015-06-25 21:21 ` Junio C Hamano
2015-06-25 22:29 ` Junio C Hamano [this message]
2015-06-26 8:06 ` Johannes Schindelin
2015-06-26 15:52 ` Jeff King
2015-06-26 17:37 ` Junio C Hamano
2015-06-27 8:57 ` Johannes Schindelin
2015-06-27 18:36 ` Junio C Hamano
2015-06-28 18:18 ` [PATCH] fsck: it is OK for a tag and a commit to lack the body Junio C Hamano
2015-06-28 18:21 ` Eric Sunshine
2015-06-29 5:12 ` Johannes Schindelin
2015-06-29 5:42 ` Junio C Hamano
2015-06-29 14:51 ` Johannes Schindelin
2015-06-25 20:48 ` git error in tag ...: unterminated header Junio C Hamano
2015-06-25 17:38 ` Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqqbng3wheu.fsf@gitster.dls.corp.google.com \
--to=gitster@pobox.com \
--cc=git@vger.kernel.org \
--cc=johannes.schindelin@gmx.de \
--cc=wd@denx.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).