You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@hive.apache.org by "Denys Kuzmenko (Jira)" <ji...@apache.org> on 2023/12/06 13:45:00 UTC
[jira] [Updated] (HIVE-27497) Vector groupby operation produces incorrect result when malicious configuration is intentionally set
[ https://issues.apache.org/jira/browse/HIVE-27497?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Denys Kuzmenko updated HIVE-27497:
----------------------------------
Priority: Major (was: Critical)
> Vector groupby operation produces incorrect result when malicious configuration is intentionally set
> ----------------------------------------------------------------------------------------------------
>
> Key: HIVE-27497
> URL: https://issues.apache.org/jira/browse/HIVE-27497
> Project: Hive
> Issue Type: Bug
> Components: Hive
> Reporter: ConfX
> Priority: Major
> Attachments: reproduce.sh
>
>
> h2. What happened:
> Hive VectorGroupByOperator produces incorrect groupby results when configuration parameter hive.vectorized.groupby.maxentries and hive.groupby.mapaggr.checkinterval are set to some bad value, for example, negative value. When Hive first in hash mode processing aggregation, the malicious configuration can forcibly change processing mode to streaming mode, and the cols in hash mode will be flushed out and not aggregate with the cols in streaming mode, which makes the groupby result incorrect.
> This issue is very severe because an attacker can control the value of these 2 parameters to control the malicious behavior; also there should have some value checkers for such parameters, at least a negative value checker for these 2 parameters.
> h2. Buggy code:
> In hash mode, the vector groupby operator checks hash efficiency at the end of each processBatch.
> {code:java}
> @Override
> public void processBatch(VectorizedRowBatch batch) throws HiveException {
> ...
> if (this instanceof ProcessingModeHashAggregate) {
> // Check if we should turn into streaming mode
> ((ProcessingModeHashAggregate)this).checkHashModeEfficiency();
> }
> }
> {code}
> In checkHashModeEfficiency, there are three if-conditions that control whether to switch to streaming mode.
> {code:java}
> private void checkHashModeEfficiency() throws HiveException {
> if (lastModeCheckRowCount > numRowsCompareHashAggr) {
> ...
> final long inputRecords = sumBatchSize;
> final long outputRecords = numEntriesHashTable + numFlushedOutEntriesBeforeFinalFlush;
> final float ratio = (outputRecords) / (inputRecords * 1.0f);
> if (ratio > minReductionHashAggr) {
> if (inputRecords > maxHtEntries) {
> flush(true);
> changeToStreamingMode();
> }
> }
> }
> }
> {code}
> Here numRowsCompareHashAggr is get from hive.groupby.mapaggr.checkinterval; minReductionHashAggr is get from hive.vectorized.groupby.maxentries. If these two configuration parameters are set to small enough values, e.g. negative values, then these 2 if statements are very likely to be always true. In this way, the flush will be called and changed to streaming mode.
> This mode changing somehow leads to incorrect grouby aggregation.
> Take a look at the following example, we have:
> | type | | | | | | | |
> | int | null | 1 | 1 | null | 2 | 2 | null |
> | string | A | A | A | C | null | null | A |
> | int | null | 2 | 2 | null | 2 | 2 | null |
> | double | 1.0 | 2.0 | 4.0 | 8.0 | 16.0 | 32.0 | 64.0 |
> Now if we want to do a sum, the expected result would be:
> | type | | | | |
> | int | 1 | 2 | null | null |
> | string | A | null | A | C |
> | int | 2 | 2 | null | null |
> | double | 6.0 | 48.0 | 65.0 | 8.0 |
> However, if we intentionally set hive.vectorized.groupby.maxentries to -84396285 and hive.groupby.mapaggr.checkinterval to -1152319368, the output becomes:
> | type | | | | | | |
> | int | 1 | null | 1 | null | 2 | null |
> | string | A | A | A | C | null | A |
> | int | 2 | null | 2 | null | 2 | null |
> | double | 2.0 | 1.0 | 4.0 | 8.0 | 48.0 | 46.0 |
> This is due to the fact that in the time in processBatch, checkHashModeEfficiency do flush and change to streaming, and the first two cols do not do the sum operation with the later cols. Later cols do the sum operation in streaming mode partially correct.
> h2. How to reproduce:
> The above example is taken from a test in Hive ql module called org.apache.hadoop.hive.ql.exec.vector.TestVectorGroupByOperator#testMultiKeyIntStringInt, you can follow the steps to reproduce the bug:
> (1) Set hive.vectorized.groupby.maxentries to -84396285 and hive.groupby.mapaggr.checkinterval to -1152319368
> (2) Run test org.apache.hadoop.hive.ql.exec.vector.TestVectorGroupByOperator#testMultiKeyIntStringInt.
> For an easy reproduction, run the reproduce.sh in the attachment.
> You should observe the following failure:
> {code}
> java.lang.AssertionError: [1, A, 2] expected:<6.0> but was:<2.0>
> at org.junit.Assert.fail(Assert.java:89)
> at org.junit.Assert.failNotEquals(Assert.java:835)
> at org.junit.Assert.assertEquals(Assert.java:120)
> at org.apache.hadoop.hive.ql.exec.vector.TestVectorGroupByOperator$ValueValidator.validate(TestVectorGroupByOperator.java:2773)
> at org.apache.hadoop.hive.ql.exec.vector.TestVectorGroupByOperator$12.inspectRow(TestVectorGroupByOperator.java:2463)
> at org.apache.hadoop.hive.ql.exec.vector.util.FakeCaptureOutputOperator.process(FakeCaptureOutputOperator.java:94)
> at org.apache.hadoop.hive.ql.exec.vector.util.FakeCaptureVectorToRowOutputOperator.process(FakeCaptureVectorToRowOutputOperator.java:162)
> at org.apache.hadoop.hive.ql.exec.Operator.vectorForward(Operator.java:919)
> at org.apache.hadoop.hive.ql.exec.vector.VectorGroupByOperator.flushOutput(VectorGroupByOperator.java:1347)
> at org.apache.hadoop.hive.ql.exec.vector.VectorGroupByOperator.closeOp(VectorGroupByOperator.java:1355)
> at org.apache.hadoop.hive.ql.exec.Operator.close(Operator.java:686)
> at org.apache.hadoop.hive.ql.exec.vector.TestVectorGroupByOperator.testMultiKey(TestVectorGroupByOperator.java:2479)
> at org.apache.hadoop.hive.ql.exec.vector.TestVectorGroupByOperator.testMultiKeyIntStringInt(TestVectorGroupByOperator.java:896)
> {code}
> We are happy to provide a patch if this issue is confirmed.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)