openbsc[master]: sgsn: fix problem with leading-zero-IMSIs

Harald Welte gerrit-no-reply at lists.osmocom.org
Tue Feb 28 18:20:57 UTC 2017


Patch Set 1: Code-Review-2

(1 comment)

https://gerrit.osmocom.org/#/c/1938/1/openbsc/src/gprs/sgsn_vty.c
File openbsc/src/gprs/sgsn_vty.c:

Line 595: 	sprintf(imsi_sanitized,"%015li", atol(argv[1]));
please always use snprintf to ensure we're not overflowing the buffer here.  If a number requires more digits than given in the format string, it will attempt to use more digits, so this is not a sufficient safeguard.

Also, 'atol()' converts to 'long', which may very well be only 32bits depending on the architectures.  And 32bits is only ~ 10 digits. See https://software.intel.com/en-us/articles/size-of-long-integer-type-on-different-architecture-and-os

atoll() might be more safe, or even strtoull() as it is even unsigned.  But then, it mgih be best to do this all in the 'string' domain to avoid any integer overflow related issues.


-- 
To view, visit https://gerrit.osmocom.org/1938
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I56ba0da61978bbdce71d0e320166c52b20b42517
Gerrit-PatchSet: 1
Gerrit-Project: openbsc
Gerrit-Branch: master
Gerrit-Owner: dexter <pmaier at sysmocom.de>
Gerrit-Reviewer: Harald Welte <laforge at gnumonks.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-HasComments: Yes


More information about the gerrit-log mailing list