Bug 241010 - netipsec: key_dup_keymsg bcopy too much bytes
Summary: netipsec: key_dup_keymsg bcopy too much bytes
Status: Open
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: CURRENT
Hardware: Any Any
: --- Affects Some People
Assignee: freebsd-net (Nobody)
URL:
Keywords: needs-qa
Depends on:
Blocks:
 
Reported: 2019-10-02 13:27 UTC by Jean-François Hren
Modified: 2019-10-04 02:26 UTC (History)
3 users (show)

See Also:
koobs: maintainer-feedback+
koobs: maintainer-feedback? (ae)


Attachments
fix using src->sadb_key_bits (1.49 KB, patch)
2019-10-02 13:27 UTC, Jean-François Hren
no flags Details | Diff
check and use sadb_key_bits (1.88 KB, patch)
2019-10-03 15:29 UTC, Jean-François Hren
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jean-François Hren 2019-10-02 13:27:37 UTC
Created attachment 208030 [details]
fix using src->sadb_key_bits

In the function key_dup_keymsg() of netipsec/key.c, the parameter len is the length of the SADB extension (either SADB_EXT_KEY_AUTH or SADB_EXT_KEY_ENCRYPT) not the length of the key. The real length used by the second malloc and the bcopy should be either (len - sizeof(struct sadb_key)) or (src->sadb_key_bits >> 3).
Comment 1 Eugene Grosbein freebsd_committer freebsd_triage 2019-10-02 21:24:54 UTC
Andrey, can you please take a look?
Comment 2 Andrey V. Elsukov freebsd_committer freebsd_triage 2019-10-03 09:58:24 UTC
In general your approach looks correct, but I think you need to validate that bits field will not lead to out of the bounds access before trusting user's data and doing bcopy.
Also, since this field was not checked properly in the past, it is possible that some IKE software doesn't fill it properly, and such change can break some installations.
Comment 3 Jean-François Hren 2019-10-03 15:18:58 UTC
(In reply to Andrey V. Elsukov from comment #2)
A check on sadb_key_bits can be done to ensure we are not going out-of-bound. Breaking current installations should not be an issue since this length is used later to set up encryption or authentication. If set incorrectly, not much should be working.
Comment 4 Jean-François Hren 2019-10-03 15:29:13 UTC
Created attachment 208061 [details]
check and use sadb_key_bits