You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2021/11/25 14:10:49 UTC
[GitHub] [incubator-nuttx] anchao opened a new pull request #4890: [SECURITY]net/udp/icmp: correct the unreadchable handling
anchao opened a new pull request #4890:
URL: https://github.com/apache/incubator-nuttx/pull/4890
## Summary
net/udp/icmp: correct the unreadchable handling
Reference RFC1122:
https://datatracker.ietf.org/doc/html/rfc1122
https://github.com/torvalds/linux/blob/master/net/ipv4/udp.c#L2469
https://github.com/torvalds/linux/blob/master/net/ipv4/udp.c#L2469
----------------------------------------------
RFC1122:
4.1.3 SPECIFIC ISSUES
4.1.3.1 Ports
If a datagram arrives addressed to a UDP port for which
there is no pending LISTEN call, UDP SHOULD send an ICMP
Port Unreachable message.
## Impact
UDP Destination Unreachable handling
## Testing
nmap security scan
before the patch:
```
nmap-7.92$ sudo nmap -sU -p 54310-54323 -Pn 192.168.31.240
Starting Nmap 7.80 ( https://nmap.org ) at 2021-11-25 22:09 CST
Warning: File ./nmap-services exists, but Nmap is using /usr/bin/../share/nmap/nmap-services for security and consistency reasons. set NMAPDIR=. to give priority to files in your local directory (may affect the other data files too).
Nmap scan report for 192.168.31.240
Host is up (0.00062s latency).
PORT STATE SERVICE
54310/udp open|filtered unknown
54311/udp open|filtered unknown
54312/udp open|filtered unknown
54313/udp open|filtered unknown
54314/udp open|filtered unknown
54315/udp open|filtered unknown
54316/udp open|filtered unknown
54317/udp open|filtered unknown
54318/udp open|filtered unknown
54319/udp open|filtered unknown
54320/udp open|filtered unknown
54321/udp open|filtered bo2k
54322/udp open|filtered unknown
54323/udp open|filtered unknown
MAC Address: 42:43:44:45:46:47 (Unknown)
Nmap done: 1 IP address (1 host up) scanned in 1.64 seconds
```
after the patch:
```
nmap-7.92$ sudo nmap -sU -p 54310-54323 -Pn 192.168.31.240
Starting Nmap 7.80 ( https://nmap.org ) at 2021-11-25 22:07 CST
Warning: File ./nmap-services exists, but Nmap is using /usr/bin/../share/nmap/nmap-services for security and consistency reasons. set NMAPDIR=. to give priority to files in your local directory (may affect the other data files too).
Nmap scan report for 192.168.31.240
Host is up (0.0038s latency).
PORT STATE SERVICE
54310/udp closed unknown
54311/udp closed unknown
54312/udp closed unknown
54313/udp closed unknown
54314/udp closed unknown
54315/udp closed unknown
54316/udp closed unknown
54317/udp closed unknown
54318/udp closed unknown
54319/udp closed unknown
54320/udp closed unknown
54321/udp closed bo2k
54322/udp closed unknown
54323/udp closed unknown
MAC Address: 42:43:44:45:46:47 (Unknown)
Nmap done: 1 IP address (1 host up) scanned in 0.23 seconds
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] normanr commented on a change in pull request #4890: [SECURITY]net/udp/icmp: correct the unreadchable handling
Posted by GitBox <gi...@apache.org>.
normanr commented on a change in pull request #4890:
URL: https://github.com/apache/incubator-nuttx/pull/4890#discussion_r775740083
##########
File path: include/nuttx/net/icmp.h
##########
@@ -113,8 +133,16 @@ struct icmp_hdr_s
/* ICMP_ECHO_REQUEST and ICMP_ECHO_REPLY data */
- uint16_t id; /* Used to match requests with replies */
- uint16_t seqno; /* " " "" " " " " " " " " */
+ union
+ {
+ struct
+ {
+ uint16_t id; /* Used to match requests with replies */
+ uint16_t seqno; /* " " "" " " " " " " " " */
+ };
+
+ uint32_t data;
Review comment:
This causes unaligned stores, because icmp is not guaranteed to be aligned to 4-bytes. None of the other parts of the icmp or ip headers use `unit32_t` anywhere, (rather they use `uint16_t[2]`).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] normanr commented on a change in pull request #4890: [SECURITY]net/udp/icmp: correct the unreadchable handling
Posted by GitBox <gi...@apache.org>.
normanr commented on a change in pull request #4890:
URL: https://github.com/apache/incubator-nuttx/pull/4890#discussion_r775751380
##########
File path: include/nuttx/net/icmp.h
##########
@@ -113,8 +133,16 @@ struct icmp_hdr_s
/* ICMP_ECHO_REQUEST and ICMP_ECHO_REPLY data */
- uint16_t id; /* Used to match requests with replies */
- uint16_t seqno; /* " " "" " " " " " " " " */
+ union
+ {
+ struct
+ {
+ uint16_t id; /* Used to match requests with replies */
+ uint16_t seqno; /* " " "" " " " " " " " " */
+ };
+
+ uint32_t data;
Review comment:
https://github.com/apache/incubator-nuttx/pull/5090 to fix.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 merged pull request #4890: [SECURITY]net/udp/icmp: correct the unreadchable handling
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 merged pull request #4890:
URL: https://github.com/apache/incubator-nuttx/pull/4890
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org