0

When I use strtol function to parse long values out of such lines of a file,

ffffffff8105b4a5 t send_signal

it just returns ffffffff instead of ffffffff8105b4a5!!!

Here's the code:

 uint64_t value1;
 uint64_t value2;
 char *temp = (char *) malloc(100);
fgets(temp,512, fp);
strncpy(line1,temp,16);
value1 = strtol(line1,NULL,16);
printf("str_hex1 = %x\n",value1);
printf("str_hex2 = %x\n",value2);
Maxim Egorushkin
  • 131,725
  • 17
  • 180
  • 271
Peggy
  • 639
  • 9
  • 28
  • 2
    Your code contains a buffer overflow vulnerability: `temp = malloc(100); fgets(temp, 512, …);` Consider using `fscanf(fp, "%llx", &value1)` instead. This will also help parsing the rest of the line(s). – David Foerster Jul 16 '13 at 22:25
  • 1
    @David Foerster: You should make that an answer. That is the best I've seen thus far, though mentioning `` macros like SCNu64 might be beneficial since `uint64_t` is already in the code. :-) –  Jul 17 '13 at 01:40
  • `line1` is **not** a _string_ as it lacks a _null character_ leading to UB in `strtol(line1,NULL,16);` – chux - Reinstate Monica Jun 16 '21 at 22:04

3 Answers3

2

I made an answer from my comment following Chrono's suggestion:

Buffer overflow vulnerability

If you allocate 100 bytes as I/O buffer, you should tell that to the function filling it:

char *temp = malloc(100);
fgets(temp, 100, fp);

or if you discard that buffer before returning:

char temp[100];
fgets(temp, sizeof(temp), fp);

Unnecessary copy

Why not simply

strtol(temp, NULL, 16);

Use fscanf(3) and format strings to parse streams

To parse lines like

ffffffff8105b4a5 t send_signal

I would write something similar to

#include <inttypes.h>

int rv;
uint64_t col1;
char col2;
char col3[64];

rv = fscanf(fp, "%"SCNx64" %c %63s", &col1, &col2, col3);
if (rv != 3) {
    /* error treatment */
}

This is a lot more concise than a series of fgets, strtoul and strcpy. It also saves a few memory copy operations, because it operates directly on the FILE* buffer.

Furthermore, for situations as with col3, GNU (and upcoming POSIX.1) *scanf has a conversion format extension "%ms" that allocates the necessary buffer for strings so you don't have to (and don't run into buffer overflows). Remember calling free() on that though.

David Foerster
  • 1,461
  • 1
  • 14
  • 23
1

It would appear that you have a configuration where sizeof(long) == 4 (i.e. 32-bit long). You might want to look into strtoull()/strtoll() instead of strtoul()/strtol(), and use [unsigned] long long variables instead...

Edit: actually, never mind the [unsigned] long long bit, as you already have uint64_t...

twalberg
  • 59,951
  • 11
  • 89
  • 84
-1

It seems that strtol is out of range (0xffffffff or greater is ULONG_MAX according to limits.h and the definition of strtol). Although the passed value (8105b4a5) is smaller than 0xffffffff it is bigger than LONG_MAX (system dependent), which is somehow the biggest number strtol can deal with.

Due to the fact that you are using unsigned longs the function strtoul (see definition) might be the more appropriate candidate.

To ensure that it is because of this range problem, please check your local limit.h.

Hope that works.

*Jost

Jost
  • 1,549
  • 12
  • 18
  • ULONG_MAX is not the same on all systems; `0xffffffff` is the *minimum* value it can have, but it must be the largest value that can fit in a `unsigned long`. In an environment where `long` is a 64-bit type, `ULONG_MAX` will be `0xffffffffffffffff`. – rici Jul 16 '13 at 15:14
  • @rici Yes you are right the limits differ depending which system you are on - and yes I have somehow overseen that it also can be greater. Thats why i link the origin of my information. Thanks for your hint. – Jost Jul 16 '13 at 17:01