Skip to content
This repository has been archived by the owner on Jun 1, 2022. It is now read-only.

Commit

Permalink
Check pread return values. Misc cleanup
Browse files Browse the repository at this point in the history
  • Loading branch information
@oschwald
oschwald committed Oct 30, 2015
1 parent 080b757 commit fda002b
Show file tree
Hide file tree
Showing 6 changed files with 64 additions and 33 deletions.
2 changes: 2 additions & 0 deletions .travis.yml
View file
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
sudo: false

language: c

compiler:
Expand Down
7 changes: 7 additions & 0 deletions ChangeLog
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,13 @@
* Fixed a MSVC parser stack overflow when parsing `regionName.c` and
`timeZone.c`. Fix by elliotlo. GitHub #64.
* Updated region codes and timezones.
* When using `GEOIP_MEMORY_CACHE` with an invalid database file, the search
tree traversal could attempt to read memory outside of the memory allocated
for the memory cache, resulting in a segmentation fault. A check was added
to ensure that the traversal code does not try to read beyond the end of the
file, whether in memory, memory mapped, or on disk.
* Previously the return values from file reads were ignored. We now check
these values to ensure that there were no errors.


1.6.6 2015-07-28
Expand Down
1 change: 0 additions & 1 deletion apps/geoiplookup6.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,6 @@ geoiplookup(GeoIP * gi, char *hostname, int i)
printf("%s: IP Address not found\n", GeoIPDBDescription[i]);
}else {
printf("%s: %s\n", GeoIPDBDescription[i], asnum_name);
// _say_range_by_ip(gi, ipnum);
}
}else if (GEOIP_CITY_EDITION_REV0_V6 == i) {
gir = GeoIP_record_by_ipnum_v6(gi, ipnum);
Expand Down
83 changes: 53 additions & 30 deletions libGeoIP/GeoIP.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -872,7 +872,7 @@ char * _GeoIP_iso_8859_1__utf8(const char * iso)
if (p) {
while ( ( c = *iso++ ) ) {
if (c < 0) {
k = 0xc2;
k = (char) 0xc2;
if (c >= -64) {
k++;
}
Expand Down Expand Up @@ -959,7 +959,7 @@ static void _setup_segments(GeoIP * gi)
int i, j, segment_record_length;
unsigned char delim[3];
unsigned char buf[LARGE_SEGMENT_RECORD_LENGTH];
ssize_t silence _UNUSED;

int fno = fileno(gi->GeoIPDatabase);

gi->databaseSegments = NULL;
Expand All @@ -971,9 +971,13 @@ static void _setup_segments(GeoIP * gi)
return;
}
for (i = 0; i < STRUCTURE_INFO_MAX_SIZE; i++) {
silence = read(fno, delim, 3);
if (read(fno, delim, 3) != 3) {
return;
}
if (delim[0] == 255 && delim[1] == 255 && delim[2] == 255) {
silence = read(fno, &gi->databaseType, 1 );
if (read(fno, &gi->databaseType, 1) != 1) {
return;
}
if (gi->databaseType >= 106) {
/* backwards compatibility with databases from April 2003 and earlier */
gi->databaseType -= 105;
Expand Down Expand Up @@ -1027,7 +1031,12 @@ static void _setup_segments(GeoIP * gi)
gi->databaseSegments[0] = 0;
segment_record_length = SEGMENT_RECORD_LENGTH;

silence = read(fno, buf, segment_record_length );
if (read(fno, buf,
segment_record_length) != segment_record_length) {
free(gi->databaseSegments);
gi->databaseSegments = NULL;
return;
}
for (j = 0; j < segment_record_length; j++) {
gi->databaseSegments[0] += (buf[j] << (j * 8));
}
Expand Down Expand Up @@ -1206,7 +1215,7 @@ void _check_mtime(GeoIP *gi)
gi->index_cache, sizeof(unsigned char) * idx_size );
if (gi->index_cache != NULL) {
if (pread(fileno(gi->GeoIPDatabase), gi->index_cache,
idx_size, 0 ) != idx_size) {
idx_size, 0) != idx_size) {
DEBUG_MSGF(
gi->flags,
"Error reading file %s where reloading\n",
Expand Down Expand Up @@ -1234,22 +1243,26 @@ unsigned int _GeoIP_seek_record_v6_gl(GeoIP *gi, geoipv6_t ipnum,

const unsigned char * p;
int j;
ssize_t silence _UNUSED;
int fno = fileno(gi->GeoIPDatabase);

unsigned int record_pair_length = gi->record_length * 2;

_check_mtime(gi);
if (GeoIP_teredo(gi) ) {
__GEOIP_PREPARE_TEREDO(&ipnum);
}
for (depth = 127; depth >= 0; depth--) {
unsigned int byte_offset = gi->record_length * 2 * offset;
if (byte_offset > gi->size - gi->record_length * 2) {
// The pointer is invalid
unsigned int byte_offset = record_pair_length * offset;
if (byte_offset > gi->size - record_pair_length) {
/* The pointer is invalid */
break;
}
if (gi->cache == NULL && gi->index_cache == NULL) {
/* read from disk */
silence = pread(fno, stack_buffer, gi->record_length * 2,
(long)byte_offset );
if (pread(fno, stack_buffer, record_pair_length,
(long)byte_offset) != record_pair_length) {
break;
}
} else if (gi->index_cache == NULL) {
/* simply point to record in memory */
buf = gi->cache + (long)byte_offset;
Expand Down Expand Up @@ -1330,22 +1343,26 @@ unsigned int _GeoIP_seek_record_gl(GeoIP *gi, unsigned long ipnum,
unsigned char stack_buffer[2 * MAX_RECORD_LENGTH];
const unsigned char *buf = (gi->cache == NULL) ? stack_buffer : NULL;
unsigned int offset = 0;
ssize_t silence _UNUSED;

const unsigned char * p;
int j;
int fno = fileno(gi->GeoIPDatabase);

unsigned int record_pair_length = gi->record_length * 2;

_check_mtime(gi);
for (depth = 31; depth >= 0; depth--) {
unsigned int byte_offset = gi->record_length * 2 * offset;
if (byte_offset > gi->size - gi->record_length * 2) {
// The pointer is invalid
unsigned int byte_offset = record_pair_length * offset;
if (byte_offset > gi->size - record_pair_length) {
/* The pointer is invalid */
break;
}
if (gi->cache == NULL && gi->index_cache == NULL) {
/* read from disk */
silence = pread(fno, stack_buffer, gi->record_length * 2,
byte_offset);
if (pread(fno, stack_buffer, record_pair_length,
byte_offset) != record_pair_length) {
break;
}
} else if (gi->index_cache == NULL) {
/* simply point to record in memory */
buf = gi->cache + (long)byte_offset;
Expand Down Expand Up @@ -2010,7 +2027,7 @@ char *GeoIP_database_info(GeoIP * gi)
unsigned char buf[3];
char *retval;
int hasStructureInfo = 0;
ssize_t silence _UNUSED;

int fno;

if (gi == NULL) {
Expand All @@ -2026,7 +2043,9 @@ char *GeoIP_database_info(GeoIP * gi)

/* first get past the database structure information */
for (i = 0; i < STRUCTURE_INFO_MAX_SIZE; i++) {
silence = read(fno, buf, 3 );
if (read(fno, buf, 3 ) != 3) {
return NULL;
}
if (buf[0] == 255 && buf[1] == 255 && buf[2] == 255) {
hasStructureInfo = 1;
break;
Expand All @@ -2047,13 +2066,17 @@ char *GeoIP_database_info(GeoIP * gi)
}

for (i = 0; i < DATABASE_INFO_MAX_SIZE; i++) {
silence = read(fno, buf, 3 );
if (read(fno, buf, 3 ) != 3) {
return NULL;
}
if (buf[0] == 0 && buf[1] == 0 && buf[2] == 0) {
retval = malloc(sizeof(char) * (i + 1));
if (retval == NULL) {
return NULL;
}
silence = read(fno, retval, i);
if (read(fno, retval, i) != i) {
return NULL;
}
retval[i] = '\0';
return retval;
}
Expand Down Expand Up @@ -2321,7 +2344,6 @@ char *_get_name_gl(GeoIP * gi, unsigned long ipnum, GeoIPLookup * gl)
char * org_buf, * buf_pointer;
int record_pointer;
size_t len;
ssize_t silence _UNUSED;

if (gi->databaseType != GEOIP_ORG_EDITION &&
gi->databaseType != GEOIP_ISP_EDITION &&
Expand Down Expand Up @@ -2352,9 +2374,10 @@ char *_get_name_gl(GeoIP * gi, unsigned long ipnum, GeoIPLookup * gl)
(2 * gi->record_length - 1) * gi->databaseSegments[0];

if (gi->cache == NULL) {
silence = pread(fileno(
gi->GeoIPDatabase), buf, MAX_ORG_RECORD_LENGTH,
record_pointer);
if (pread(fileno(gi->GeoIPDatabase), buf, MAX_ORG_RECORD_LENGTH,
record_pointer) == -1) {
return NULL;
}
if (gi->charset == GEOIP_CHARSET_UTF8) {
org_buf = _GeoIP_iso_8859_1__utf8( (const char * )buf );
} else {
Expand Down Expand Up @@ -2383,7 +2406,6 @@ char *_get_name_v6_gl(GeoIP * gi, geoipv6_t ipnum, GeoIPLookup * gl)
char * org_buf, * buf_pointer;
int record_pointer;
size_t len;
ssize_t silence _UNUSED;

if (
gi->databaseType != GEOIP_ORG_EDITION_V6 &&
Expand Down Expand Up @@ -2411,9 +2433,10 @@ char *_get_name_v6_gl(GeoIP * gi, geoipv6_t ipnum, GeoIPLookup * gl)
(2 * gi->record_length - 1) * gi->databaseSegments[0];

if (gi->cache == NULL) {
silence = pread(fileno(
gi->GeoIPDatabase), buf, MAX_ORG_RECORD_LENGTH,
record_pointer);
if (pread(fileno(gi->GeoIPDatabase), buf, MAX_ORG_RECORD_LENGTH,
record_pointer) == -1) {
return NULL;
}
buf[MAX_ORG_RECORD_LENGTH] = 0;
if (gi->charset == GEOIP_CHARSET_UTF8) {
org_buf = _GeoIP_iso_8859_1__utf8( (const char * )buf );
Expand Down
2 changes: 1 addition & 1 deletion libGeoIP/GeoIPCity.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ _extract_record(GeoIP * gi, unsigned int seek_record, int *next_record_ptr)
bytes_read = pread(fileno(
gi->GeoIPDatabase), record_buf,
FULL_RECORD_LENGTH, record_pointer);
if (bytes_read == 0) {
if (bytes_read <= 0) {
/* eof or other error */
free(begin_record_buf);
free(record);
Expand Down
2 changes: 1 addition & 1 deletion test/test-geoip-invalid-file.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ int main()
{
GeoIP *gi = GeoIP_open(SRCDIR "/README.md", GEOIP_MEMORY_CACHE);

// We don't detect invalid files at load, unfortunately.
/* We don't detect invalid files at load, unfortunately. */
if (gi == NULL) {
fprintf(stderr, "Error opening database\n");
return 1;
Expand Down

0 comments on commit fda002b

Please sign in to comment.