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

fat, read_clusters: Don't allow memory overflow #105

Open
perlun opened this issue Oct 13, 2017 · 0 comments
Open

fat, read_clusters: Don't allow memory overflow #105

perlun opened this issue Oct 13, 2017 · 0 comments
Labels

Comments

@perlun
Copy link
Contributor

perlun commented Oct 13, 2017

This code is a bit too naïve:

uint32_t read_clusters(fat_info_type *fat_info, void *output, uint32_t start_cluster, uint32_t skip, uint32_t number_of_clusters)
{
uint32_t cluster_number = start_cluster;
uint32_t clusters_read = 0;
do
{
if (skip > 0)
{
skip--;
}
else
{
// log_print_formatted (&log_structure, LOG_URGENCY_DEBUG,
// "Reading cluster number %lu", cluster_number);
read_single_cluster(fat_info, cluster_number, (void *)
((uint32_t) output + (clusters_read *
fat_info->bytes_per_sector *
fat_info->sectors_per_cluster)));
clusters_read++;
}
cluster_number = get_next_cluster(cluster_number, fat_info->fat, fat_info->bits);
} while (cluster_number != MAX_uint32_t && clusters_read < number_of_clusters);
return cluster_number;
}

It takes in a buffer (the output variable), but no length of the buffer. So the method has no way to prevent itself from overflowing the buffer if things go wrong...

Also, it has no way to signal to the caller that it did overflow. We would need to fix both these things. Right now, I am getting page faults because of this:

image

Something is messing with the cluster linking, so it reads the same cluster over and over again, leading to the buffer being overflowed.

@perlun perlun added the bug label Oct 13, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant