Possible issue in iproute2 package
Hi all I'm Jose Guzman from a security team at Intel. We're using iproute2 in a GNU-Linux project and I'm analyzing the code to try to find possible issues/gaps/risks. Since I'm not too familiar with the package yet I have a question about a particular piece of code that could result in a memory corruption: Version: 4.0.0 File: misc/ss.c Function: static void tcp_show_info(...) Line: ~1903 Description: There is a memory allocation for a "s.cong_alg" variable: s.cong_alg = malloc(strlen(cong_attr + 1)); The length is calculated about next position of the starting character. But next line there is a copy of the whole content: strcpy(s.cong_alg, cong_attr); I think there is a mistake and it should be something like: s.cong_alg = malloc(strlen(cong_attr) + 1); Is this the case? Is it a real bug? Also I don't see any checking for the value returned by the malloc call, what if it returns a NULL pointer? Also I found something similar about line 1903: s.cong_alg = malloc(strlen(cong_attr + 1)); strcpy(s.cong_alg, cong_attr); And another possible issue that I found: File: tc/tc_util.c Function: void print_rate(char *buf, int len, __u64 rate) Line: ~264 In the case that user inputs a high value for rate, the "for" loop will exit in the condition meaning that variable "i" get the value of 5 which will be an invalid index for the "units" array due to that array has only 5 elements. I hope you can help me by checking these issues and tell me whether they are real issues or not since you know much better the code. Also I don't know if you have already this reported, I didn't find a list of issues for this package. Can you tell me where is such list? I really appreciate any help on this. Thanks in advance. Jose G.
Re: Possible issue in iproute2 package
Hi Jose, thanks for your report! On 05/28/2015 11:12 PM, Guzman Mosqueda, Jose R wrote: ... We're using iproute2 in a GNU-Linux project and I'm analyzing the code to try to find possible issues/gaps/risks. Since I'm not too familiar with the package yet I have a question about a particular piece of code that could result in a memory corruption: Version: 4.0.0 File: misc/ss.c Function: static void tcp_show_info(...) Line: ~1903 Description: There is a memory allocation for a "s.cong_alg" variable: s.cong_alg = malloc(strlen(cong_attr + 1)); The length is calculated about next position of the starting character. But next line there is a copy of the whole content: strcpy(s.cong_alg, cong_attr); I think there is a mistake and it should be something like: s.cong_alg = malloc(strlen(cong_attr) + 1); Is this the case? Is it a real bug? Also I don't see any checking for the value returned by the malloc call, what if it returns a NULL pointer? Cc'ing Vadim for ... commit 8250bc9ff4e55a3ef397ed8c7612f1392d164295 Author: Vadim Kochan Date: Tue Jan 20 16:14:24 2015 +0200 ss: Unify inet sockets output Signed-off-by: Vadim Kochan Also I found something similar about line 1903: s.cong_alg = malloc(strlen(cong_attr + 1)); strcpy(s.cong_alg, cong_attr); And another possible issue that I found: File: tc/tc_util.c Function: void print_rate(char *buf, int len, __u64 rate) Line: ~264 In the case that user inputs a high value for rate, the "for" loop will exit in the condition meaning that variable "i" get the value of 5 which will be an invalid index for the "units" array due to that array has only 5 elements. I hope you can help me by checking these issues and tell me whether they are real issues or not since you know much better the code. Also I don't know if you have already this reported, I didn't find a list of issues for this package. Can you tell me where is such list? I really appreciate any help on this. Thanks in advance. Jose G. N�r��y���b�X��ǧv�^�){.n�+���z�^�)���w*jg����ݢj/���z�ޖ��2�ޙ���&�)ߡ�a�����G���h��j:+v���w�٥ -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Possible issue in iproute2 package
Hi Jose, On Thu, May 28, 2015 at 09:12:15PM +, Guzman Mosqueda, Jose R wrote: > > Hi all > > I'm Jose Guzman from a security team at Intel. > We're using iproute2 in a GNU-Linux project and I'm analyzing the code > to try to find possible issues/gaps/risks. > Since I'm not too familiar with the package yet I have a question about > a particular piece of code that could result in a memory corruption: > > Version: 4.0.0 > File: misc/ss.c > Function: static void tcp_show_info(...) > Line: ~1903 > Description: There is a memory allocation for a "s.cong_alg" variable: > s.cong_alg = malloc(strlen(cong_attr + 1)); > The length is calculated about next position of the starting character. > But next line there is a copy of the whole content: > strcpy(s.cong_alg, cong_attr); > I think there is a mistake and it should be something like: > s.cong_alg = malloc(strlen(cong_attr) + 1); I think strdup can be used here. I will send a patch. Thank You! -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html