Skip to content
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.

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

Added cmake support and replaced unsafe string functions with safe ones #25

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

chrysante
Copy link

I modified the library to make it fit for my use and I thought I share it here.

The name and linkname fields in the mtar_header_t struct are both 100 bytes long as mandated by the tar format. The tar format does allow longer names but this seems not supported by this library. Filename size is not checked by the library resulting in corrupted archives if a longer name is specified. I changed the behaviour to printing an error message and calling abort() if a name longer than 100 characters (including null terminator) is specified.

While I was at it I also changed all other string functions that are considered unsafe to their safe alternatives.

@chrysante chrysante marked this pull request as ready for review March 12, 2024 11:18
sscanf(rh->owner, "%o", &h->owner);
sscanf(rh->size, "%o", &h->size);
sscanf(rh->mtime, "%o", &h->mtime);
sscanf(rh->mode, "%8o", &h->mode);
Copy link

Choose a reason for hiding this comment

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

You don't need to specify octal size for h->mode, h->owner, and h->mtime. They're fixed values and guaranteed will not "overflow" from it's range.

src/microtar.c Outdated
strcpy(h->linkname, rh->linkname);

// Here we can memcpy because both buffers have the same size
memcpy(h->name, rh->name, name_buf_width);
Copy link

Choose a reason for hiding this comment

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

Use sizeof for array here for either h->name or rh->name. They both are same sized.

sprintf(rh->owner, "%o", h->owner);
sprintf(rh->size, "%o", h->size);
sprintf(rh->mtime, "%o", h->mtime);
snprintf(rh->mode, 8, "%o", h->mode);
Copy link

Choose a reason for hiding this comment

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

Same, you don't need to explicitly specify size here. It will not overflow

src/microtar.c Outdated
@@ -333,7 +349,7 @@ int mtar_write_file_header(mtar_t *tar, const char *name, unsigned size) {
mtar_header_t h;
/* Build header */
memset(&h, 0, sizeof(h));
strcpy(h.name, name);
guarded_strcpy(h.name, name, name_buf_width);
Copy link

Choose a reason for hiding this comment

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

Use memcpy here

@rilysh
Copy link

rilysh commented Mar 29, 2024

Considering the project activity was about 6 or 7 years ago, and since then no PRs are merged, I think you should generally avoid the hassle of creating PRs on this repository. Instead, you may wanna apply (your changes) in your local tree (project).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants